AArch64/TargetTransformInfo: Report div/rem constant immediate costs as TCC_Free

[MatzeB]

Summary

DIV/REM instructions with constant operands are replaced by a series of
muls/shifts/etc. with different constants. Unfortunately the
ConstantHoisting pass runs too early to catch this expansion and will
operate on immediate that will get changed later. Report these
operations as TCC_Free so ConstantHoisting will not touch them.

[MatzeB]

• This is not inspired by some benchmark, but from me needing stable tests in an upcoming change making DAGCombiner slightly less aggressive with OpaqueConstants. It does however seem like the right thing to do in general.

• The same transformation was done for ARM in rL266464, arguing that no effect could be measured in some unspecified benchmark on aarch64.

• With this home-made microbenchmark, I do measure more than 5x perf improvement:

#include <stdint.h>

#define SIZE 100000

void __attribute__((noinline)) divfoo(int64_t *signed_arr, uint64_t *unsigned_arr) {
    for (unsigned i = 0; i < SIZE; ++i) {
        signed_arr[i] /= 0xcafebabe5;
        unsigned_arr[i] /= 0xcafebabe5;
    }
}

int main(void) {
    static int64_t signed_arr[SIZE];
    static uint64_t unsigned_arr[SIZE];
    for (unsigned i = 0; i < 1000; ++i)
        divfoo(signed_arr, unsigned_arr);
}

[MatzeB]

ping

lib/Target/AArch64/AArch64TargetTransformInfo.cpp

int AArch64TTIImpl::getIntImmCost(unsigned Opcode, unsigned Idx,
   case Instruction::GetElementPtr:
     // Always hoist the base address of a GetElementPtr.
     if (Idx == 0)
       return 2 * TTI::TCC_Basic;
     return TTI::TCC_Free;
   case Instruction::Store:
     ImmIdx = 0;
     break;
-  case Instruction::Add:
-  case Instruction::Sub:
-  case Instruction::Mul:
   case Instruction::UDiv:
   case Instruction::SDiv:
   case Instruction::URem:
   case Instruction::SRem:
+    // Division by constant is typically expanded later into a different
+    // instruction sequence. This completely changes the constants.
+    // Report them as "free" to stop ConstantHoist from marking them as opaque.
+    return TTI::TCC_Free;
+  case Instruction::Add:
+  case Instruction::Sub:
+  case Instruction::Mul:
   case Instruction::And:
   case Instruction::Or:
   case Instruction::Xor:
   case Instruction::ICmp:
     ImmIdx = 1;
     break;
   // Always return TCC_Free for the shift value of a shift instruction.
   case Instruction::Shl:

test/Transforms/ConstantHoisting/AArch64/bad-cases.ll

+; RUN: opt -consthoist -S < %s | FileCheck %s
+target triple = "aarch64--"
+
+; We don't want to convert constant divides because the benefit from converting
+; them to a mul in the backend is larget than constant materialization savings.
+define void @signed_const_division(i32 %in1, i32 %in2, i32* %addr) {
+; CHECK-LABEL: @signed_const_division
+; CHECK: %res1 = sdiv i32 %l1, 1000000000
+; CHECK: %res2 = srem i32 %l2, 1000000000
+entry:
+  br label %loop
+
+loop:
+  %l1 = phi i32 [%res1, %loop], [%in1, %entry]
+  %l2 = phi i32 [%res2, %loop], [%in2, %entry]
+  %res1 = sdiv i32 %l1, 1000000000
+  store volatile i32 %res1, i32* %addr
+  %res2 = srem i32 %l2, 1000000000
+  store volatile i32 %res2, i32* %addr
+  %again = icmp eq i32 %res1, %res2
+  br i1 %again, label %loop, label %end
+
+end:
+  ret void
+}
+
+define void @unsigned_const_division(i32 %in1, i32 %in2, i32* %addr) {
+; CHECK-LABEL: @unsigned_const_division
+; CHECK: %res1 = udiv i32 %l1, 1000000000
+; CHECK: %res2 = urem i32 %l2, 1000000000
+
+entry:
+  br label %loop
+
+loop:
+  %l1 = phi i32 [%res1, %loop], [%in1, %entry]
+  %l2 = phi i32 [%res2, %loop], [%in2, %entry]
+  %res1 = udiv i32 %l1, 1000000000
+  store volatile i32 %res1, i32* %addr
+  %res2 = urem i32 %l2, 1000000000
+  store volatile i32 %res2, i32* %addr
+  %again = icmp eq i32 %res1, %res2
+  br i1 %again, label %loop, label %end
+
+end:
+  ret void
+}