[PATCH] D122829: [AArch64] Optimize SDIV with pow2 constant divisor

Thu Mar 31 09:45:37 PDT 2022

craig.topper added a comment.

In D122829#3419836 <https://reviews.llvm.org/D122829#3419836>, @bcl5980 wrote:

> In D122829#3419777 <https://reviews.llvm.org/D122829#3419777>, @craig.topper wrote:
>
>> Does this differ in an important way from what you get if you remove BuildSDIVPow2? I think you'll get sign bit shifted all the way to the right followed by a lsr. While this patch will shift the sign bit only as far as it needs to.
>
> remove BuildSDIVPow2 also works for scalar cases. But test failed on sve cases. 
> I can also shift 31/63 everytime but I think currect code is also correct.

Is that because of the SVE special case on line 13536? What if you left that code and returned SDValue() after it?

================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:13536
   // later. This allows us to handle larger than legal types.
   if (VT.isScalableVector() || Subtarget->useSVEForFixedLengthVectors())
     return SDValue(N, 0);
----------------
Should this be checking that the VT is a fixed vector before calling Subtarget->useSVEForFixedLengthVectors()? Without that it looks like even scalar are blocked when Subtarget->useSVEForFixedLengthVectors() is true.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122829/new/

https://reviews.llvm.org/D122829