[PATCH] D53067: [AArch64] Swap comparison operands if that enables some folding.

[Arnaud A. de Grandmaison via Phabricator via llvm-commits]

aadg added inline comments.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:1802
+      EVT VT = Op.getValueType();
+      if ((VT == MVT::i32 && Shift <= 31) || (VT == MVT::i64 && Shift <= 63))
+        return 1;
----------------
aadg wrote:
> SjoerdMeijer wrote:
> > Had a quick first look, so sorry for some question about testing.... If the shift amount is in the range [0,31] and [0,64], are the edge cases tested (also values just outside this range)? Also, can `Shift` be negative here?
> > 
> > 
> The ARMARM definitely states Shift has to be in [0:31] (32bits variant) or [0:63] (64 bits variant), so it can not be negative.
> You however point rightfully that testcases for the out-of-range cases for the upper bound are missing --- I'll add them right away. 
> 
> 
After thinking about it a bit more, testing the out of bound case for upper range is ... UB. But there are definitely some testcases missing, and those will be added.


Repository:
  rL LLVM

https://reviews.llvm.org/D53067