[AArch64] Add patterns for zext/sext of shift amount.

[efriedma]

Summary

Not sure this is the best fix, but it saves an instruction for certain constructs involving variable shifts.

[t.p.northover]

test/CodeGen/AArch64/shift-mod.ll

21–22 ↗

(On Diff #177762)

There's no particular reason to use w8 here so this test is pretty fragile. In general I think we should never be hard-coding registers that aren't at an ABI boundary (and so that update_llc_test_checks.py is usually more trouble than it's worth).

[efriedma]

update_llc_test_checks is a useful starting point because it generates the right checks to ensure you're checking the whole function, which is inconvenient to do by hand (trying to CHECK-NOT your way to checking the whole function is a bad idea).

I can replace the explicit reference to r8 with a regex, sure. (We could look at modifying update_llc_test_checks to generate a regex where appropriate, I guess... but it might be tricky to distinguish when a value is ABI-significant.)

[t.p.northover]

update_llc_test_checks is a useful starting point because it generates the right checks to ensure you're checking the whole function, which is inconvenient to do by hand (trying to CHECK-NOT your way to checking the whole function is a bad idea).

I can see some benefit there, though when the tests do break I'm often annoyed by it checking irrelevant details (those comments in this case, for example). Either way I think we should very strongly discourage people from treating its output as good enough for final tests except in trivial one-instruction cases.

I can replace the explicit reference to r8 with a regex, sure.

Thanks.

(We could look at modifying update_llc_test_checks to generate a regex where appropriate, I guess... but it might be tricky to distinguish when a value is ABI-significant.)

I was musing along similar lines myself this morning. Intel vs AT&T (and use/def in general) would also be tricky; I suspect it would need to bake quite a bit of target-specific knowledge into it to get reasonable output.

[efriedma]

I suspect it would need to bake quite a bit of target-specific knowledge into it to get reasonable output

Yes. But that's expected; assembly is inherently target-specific. There are some target-specific checks already.

[efriedma]

Use regex instead of explicit "x8".

[t.p.northover]

Thanks! LGTM.

llvm/trunk/lib/Target/AArch64/AArch64InstrFormats.td

multiclass Shift<bits<2> shift_type, string asm, SDNode OpNode> {
   def : Pat<(i32 (OpNode GPR32:$Rn, (i64 (zext GPR32:$Rm)))),
             (!cast<Instruction>(NAME # "Wr") GPR32:$Rn, GPR32:$Rm)>;
 
   def : Pat<(i32 (OpNode GPR32:$Rn, (i64 (anyext GPR32:$Rm)))),
             (!cast<Instruction>(NAME # "Wr") GPR32:$Rn, GPR32:$Rm)>;
 
   def : Pat<(i32 (OpNode GPR32:$Rn, (i64 (sext GPR32:$Rm)))),
             (!cast<Instruction>(NAME # "Wr") GPR32:$Rn, GPR32:$Rm)>;
+
+  def : Pat<(i64 (OpNode GPR64:$Rn, (i64 (sext GPR32:$Rm)))),
+            (!cast<Instruction>(NAME # "Xr") GPR64:$Rn,
+                (SUBREG_TO_REG (i32 0), GPR32:$Rm, sub_32))>;
+
+  def : Pat<(i64 (OpNode GPR64:$Rn, (i64 (zext GPR32:$Rm)))),
+            (!cast<Instruction>(NAME # "Xr") GPR64:$Rn,
+                (SUBREG_TO_REG (i32 0), GPR32:$Rm, sub_32))>;
 }
 
 class ShiftAlias<string asm, Instruction inst, RegisterClass regtype>
     : InstAlias<asm#"\t$dst, $src1, $src2",
                 (inst regtype:$dst, regtype:$src1, regtype:$src2), 0>;
 
 class BaseMulAccum<bit isSub, bits<3> opc, RegisterClass multype,
                        RegisterClass addtype, string asm,

llvm/trunk/test/CodeGen/AArch64/shift-mod.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=aarch64 < %s | FileCheck %s
 
 ; Check that we optimize out AND instructions and ADD/SUB instructions
 ; modulo the shift size to take advantage of the implicit mod done on
 ; the shift amount value by the variable shift/rotate instructions.
 
 define i32 @test1(i32 %x, i64 %y) {
 ; CHECK-LABEL: test1:
-; CHECK-NOT: and
-; CHECK: lsr
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    lsr w0, w0, w1
+; CHECK-NEXT:    ret
   %sh_prom = trunc i64 %y to i32
   %shr = lshr i32 %x, %sh_prom
   ret i32 %shr
 }
 
 define i64 @test2(i32 %x, i64 %y) {
 ; CHECK-LABEL: test2:
-; CHECK-NOT: orr
-; CHECK-NOT: sub
-; CHECK: neg
-; CHECK: asr
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    neg w[[REG:[0-9]+]], w0
+; CHECK-NEXT:    asr x0, x1, x[[REG]]
+; CHECK-NEXT:    ret
   %sub9 = sub nsw i32 64, %x
   %sh_prom12.i = zext i32 %sub9 to i64
   %shr.i = ashr i64 %y, %sh_prom12.i
   ret i64 %shr.i
 }
 
 define i64 @test3(i64 %x, i64 %y) {
 ; CHECK-LABEL: test3:
-; CHECK-NOT: add
-; CHECK: lsl
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    lsl x0, x1, x0
+; CHECK-NEXT:    ret
   %add = add nsw i64 64, %x
   %shl = shl i64 %y, %add
   ret i64 %shl
-}
+}
+
+define i64 @test4(i64 %y, i32 %s) {
+; CHECK-LABEL: test4:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    // kill: def $w1 killed $w1 def $x1
+; CHECK-NEXT:    asr x0, x0, x1
+; CHECK-NEXT:    ret
+entry:
+  %sh_prom = zext i32 %s to i64
+  %shr = ashr i64 %y, %sh_prom
+  ret i64 %shr
+}
+
+define i64 @test5(i64 %y, i32 %s) {
+; CHECK-LABEL: test5:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    // kill: def $w1 killed $w1 def $x1
+; CHECK-NEXT:    asr x0, x0, x1
+; CHECK-NEXT:    ret
+entry:
+  %sh_prom = sext i32 %s to i64
+  %shr = ashr i64 %y, %sh_prom
+  ret i64 %shr
+}
+
+define i64 @test6(i64 %y, i32 %s) {
+; CHECK-LABEL: test6:
+; CHECK:       // %bb.0: // %entry
+; CHECK-NEXT:    // kill: def $w1 killed $w1 def $x1
+; CHECK-NEXT:    lsl x0, x0, x1
+; CHECK-NEXT:    ret
+entry:
+  %sh_prom = sext i32 %s to i64
+  %shr = shl i64 %y, %sh_prom
+  ret i64 %shr
+}