Re: [PATCH v2 00/11] aarch64: Implement TImode comparisons

[Richard Sandiford <richard.sandiford@arm.com>]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Mon, Apr 06, 2020 at 12:19:42PM +0100, Richard Sandiford wrote:
>> The reason I'm not keen on using special modes for this case is that
>> they'd describe one way in which the result can be used rather than
>> describing what the instruction actually does.  The instruction really
>> does set all four flags to useful values.  The "problem" is that they're
>> not the values associated with a compare between two values, so representing
>> them that way will always lose information.
>
> CC modes describe how the flags are set, not how the flags are used.
> You cannot easily describe the V bit setting with a compare (it needs
> a mode bigger than the register), is that what you mean?

I meant more that, if you want to represent the result of SBCS using
a compare, you have to decide ahead of time which result you're
interested in, and pick the CC mode and compare representation
accordingly.  E.g. SBCS res, x, y sets the Z flag if res is zero.
This is potentially useful, and could be represented as a
compare:CC_Z (say) between values based on x, y and the C flag.
SBCS res, x, y also sets the C flag if the first multiword value
(x and less significant words) is GEU the second.  This too is
potentially useful and could be represented as a compare:CC_C (say)
between values based on x, y and the C flag.  But these two compares
can't be the *same* compares, because SBCS can create a Z-set, C-clear
result that is impossible for single compares.

This is a case that the existing aarch64 pattern I mentioned gets wrong:

(define_insn "*usub<GPI:mode>3_carryinC"
  [(set (reg:CC CC_REGNUM)
	(compare:CC
	  (zero_extend:<DWI>
	    (match_operand:GPI 1 "register_operand" "r"))
	  (plus:<DWI>
	    (zero_extend:<DWI>
	      (match_operand:GPI 2 "register_operand" "r"))
	    (match_operand:<DWI> 3 "aarch64_borrow_operation" ""))))
   (set (match_operand:GPI 0 "register_operand" "=r")
	(minus:GPI
	  (minus:GPI (match_dup 1) (match_dup 2))
	  (match_operand:GPI 4 "aarch64_borrow_operation" "")))]
   ""
   "sbcs\\t%<w>0, %<w>1, %<w>2"
  [(set_attr "type" "adc_reg")]
)

(op3 == inverse of the carry flag).  When op0 == 0, op2 == -1 and op3 == 1,
the compare folds to:

  (compare:CC 0 (1 << 32))

for SI.  This compare ought to give an NE result, but the SBCS instead
sets the Z flag, giving an EQ result.

That's what I meant by the CC mode describing how the result is used.
You have to pick from a choice of several mutually-exclusive compare
representations depending on which result you want.  Sure, each
representation has to describe how the flags are set in an accurate way
(like the compare:CC_Z and compare:CC_C above are individually accurate).
But the choice of CC mode does reflect how the result is used, and can't
be made independently of how the result is used.

For SUBS you can pick the mode of the compare and the compared values
independently of how the result is used.  This seems cleaner and leads
to be better CSE opportunities, etc.  I think it'd be good to be able
to do the same for SBCS, whether that's through an unspec or (better)
a new rtx_code.

Thanks,
Richard