likely arm64 miscompile

[regehr]

ok this has been llvm-reduced but it's still a bit complex, sorry!

define i32 @f(i32 %x) {
  %or1 = or i32 %x, 1
  %sh1 = shl i32 %or1, 5
  %sh2 = lshr i32 %x, 27
  %1 = and i32 %sh2, 1
  %r = or i32 %sh1, %1
  ret i32 %r
}

we believe that f(0xf0000000) -> 32 and that is what we get when using global isel, and also when compiling this code for x64.

on the other hand, the default arm64 backend from top of tree gives f(0xf0000000) -> 62

we don't think there's any UB going on here, seems like a straight-up miscompile

cc @ornata @nunoplopes @ryan-berger @nbushehri @aqjune @Hatsunespica

[llvmbot]

@llvm/issue-subscribers-backend-aarch64

[regehr]

bad asm from the default backend

_f:
	ror	w8, w0, #27
	orr	w0, w8, #0x20
	ret

good asm from global isel:

_f: 
	orr	w8, w0, #0x1
	ubfx	w9, w0, #27, #1
	orr	w0, w9, w8, lsl #5
	ret

[topperc]

-mtriple=riscv32 -mattr=+zbb produces something equivalent to the bad AArch64 code.

[topperc]

Seems to be something in the rotate matching. x86 picks a funnel shift instead.

[topperc]

This patch fixes it

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 71618eb2bd7c..ac9670746c53 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -7482,7 +7482,7 @@ SDValue DAGCombiner::MatchRotate(SDValue LHS, SDValue RHS, const SDLoc &DL) {
 
   // TODO: Support pre-legalization funnel-shift by constant.
   bool IsRotate = LHSShift.getOperand(0) == RHSShift.getOperand(0);
-  if (!IsRotate && !(HasFSHL || HasFSHR)) {
+  if (!IsRotate && !(HasFSHL || HasFSHR)) {
     if (TLI.isTypeLegal(VT) && LHS.hasOneUse() && RHS.hasOneUse() &&
         ISD::matchBinaryPredicate(LHSShiftAmt, RHSShiftAmt, MatchRotateSum) &&
         !LHSMask && !RHSMask) {
       // Look for a disguised rotate by constant.

We might be able to do better. The block of code behind that if forgot to do anything with LHSMask and RHSMask so this patch makes it so we don't enter the code when we need to do something with them.

[topperc]

Candidate patch https://reviews.llvm.org/D124711

[topperc]

Fixed by 6affe87