[GISel]: Attach missing range metadata while translating G_LOADs

[aditya_nandakumar]

Summary

Attach range information to MMO while translating Loads.

[arsenm]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

892

How does this work if it's split into multiple register? I would assume this needs to be decomposed and only trivially valid if there is one result register

[aditya_nandakumar]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

892

Good point. I didn't think of that. For now I'll add the ranges only if there is one register.

[aditya_nandakumar]

Attach range information only with one def

[arsenm]

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

[aditya_nandakumar]

In D65048#1594784, @arsenm wrote:

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

1. There was no need until now.
2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.

I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.

[aditya_nandakumar]

Thanks. r366656

[arsenm]

In D65048#1594810, @aditya_nandakumar wrote:

In D65048#1594784, @arsenm wrote:

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

1. There was no need until now.
2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.

I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.

I don't know what could be significantly different

[aemerson]

In D65048#1594839, @arsenm wrote:

In D65048#1594810, @aditya_nandakumar wrote:

In D65048#1594784, @arsenm wrote:

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

1. There was no need until now.
2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.

I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.

I don't know what could be significantly different

@paquette is going to look at this from a design perspective but we haven't so far had a strong immediate need or the time to do this.

My thoughts are that computeKnownBits in SelectionDAG is a potentially very costly analysis, and we currently have magic recursion depth limits to prevent extremely long compile times. We don't have any evidence to support an alternative yet, but it would be nice to prototype different approaches for the GlobalISel equivalent. Maybe add caching support for the start, and/or implement it using a bottom up approach.

[aditya_nandakumar]

In D65048#1595839, @aemerson wrote:

In D65048#1594839, @arsenm wrote:

In D65048#1594810, @aditya_nandakumar wrote:

In D65048#1594784, @arsenm wrote:

LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work

Yes - that's how I stumbled on this.
We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because

1. There was no need until now.
2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.

I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.

I don't know what could be significantly different

@paquette is going to look at this from a design perspective but we haven't so far had a strong immediate need or the time to do this.

My thoughts are that computeKnownBits in SelectionDAG is a potentially very costly analysis, and we currently have magic recursion depth limits to prevent extremely long compile times. We don't have any evidence to support an alternative yet, but it would be nice to prototype different approaches for the GlobalISel equivalent. Maybe add caching support for the start, and/or implement it using a bottom up approach.

It should be quite trivial to add caching support to compute known bits while operating on demand similar to DAG (which is pretty much bottom up).

[arsenm]

For now, I would be happy with the interface that handles the most trivial cases so I can use it in code I’m porting so I don’t have to think about it later. Having it functionally complete is less important

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateLoad(const User &U, MachineIRBuilder &MIRBuilder) {
   if (CLI->supportSwiftError() && isSwiftError(LI.getPointerOperand())) {
     assert(Regs.size() == 1 && "swifterror should be single pointer");
     Register VReg = SwiftError.getOrCreateVRegUseAt(&LI, &MIRBuilder.getMBB(),
                                                     LI.getPointerOperand());
     MIRBuilder.buildCopy(Regs[0], VReg);
     return true;
   }
 
-
+  const MDNode *Ranges =
+      Regs.size() == 1 ? LI.getMetadata(LLVMContext::MD_range) : nullptr;
   for (unsigned i = 0; i < Regs.size(); ++i) {
     Register Addr;
     MIRBuilder.materializeGEP(Addr, Base, OffsetTy, Offsets[i] / 8);
 
     MachinePointerInfo Ptr(LI.getPointerOperand(), Offsets[i] / 8);
     unsigned BaseAlign = getMemOpAlignment(LI);
     auto MMO = MF->getMachineMemOperand(
         Ptr, Flags, (MRI->getType(Regs[i]).getSizeInBits() + 7) / 8,
-        MinAlign(BaseAlign, Offsets[i] / 8), AAMDNodes(), nullptr,
+        MinAlign(BaseAlign, Offsets[i] / 8), AAMDNodes(), Ranges,
         LI.getSyncScopeID(), LI.getOrdering());
     MIRBuilder.buildLoad(Regs[i], Addr, *MMO);
   }
 
   return true;
 }
 
 bool IRTranslator::translateStore(const User &U, MachineIRBuilder &MIRBuilder) {

llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

 ; CHECK-LABEL: name: load
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x0
 ; CHECK: [[ADDR42:%[0-9]+]]:_(p42) = COPY $x1
 ; CHECK: [[VAL1:%[0-9]+]]:_(s64) = G_LOAD [[ADDR]](p0) :: (load 8 from %ir.addr, align 16)
 ; CHECK: [[VAL2:%[0-9]+]]:_(s64) = G_LOAD [[ADDR42]](p42) :: (load 8 from %ir.addr42, addrspace 42)
 ; CHECK: [[SUM2:%.*]]:_(s64) = G_ADD [[VAL1]], [[VAL2]]
 ; CHECK: [[VAL3:%[0-9]+]]:_(s64) = G_LOAD [[ADDR]](p0) :: (volatile load 8 from %ir.addr)
 ; CHECK: [[SUM3:%[0-9]+]]:_(s64) = G_ADD [[SUM2]], [[VAL3]]
-; CHECK: $x0 = COPY [[SUM3]]
+; CHECK: [[VAL4:%[0-9]+]]:_(s64) = G_LOAD [[ADDR]](p0) :: (load 8 from %ir.addr, !range !0)
+; CHECK: [[SUM4:%[0-9]+]]:_(s64) = G_ADD [[SUM3]], [[VAL4]]
+; CHECK: $x0 = COPY [[SUM4]]
 ; CHECK: RET_ReallyLR implicit $x0
 define i64 @load(i64* %addr, i64 addrspace(42)* %addr42) {
   %val1 = load i64, i64* %addr, align 16
 
   %val2 = load i64, i64 addrspace(42)* %addr42
   %sum2 = add i64 %val1, %val2
 
   %val3 = load volatile i64, i64* %addr
   %sum3 = add i64 %sum2, %val3
-  ret i64 %sum3
+
+  %val4 = load i64, i64* %addr, !range !0
+  %sum4 = add i64 %sum3, %val4
+  ret i64 %sum4
 }
 
 ; CHECK-LABEL: name: store
 ; CHECK: [[ADDR:%[0-9]+]]:_(p0) = COPY $x0
 ; CHECK: [[ADDR42:%[0-9]+]]:_(p42) = COPY $x1
 ; CHECK: [[VAL1:%[0-9]+]]:_(s64) = COPY $x2
 ; CHECK: [[VAL2:%[0-9]+]]:_(s64) = COPY $x3
 ; CHECK: G_STORE [[VAL1]](s64), [[ADDR]](p0) :: (store 8 into %ir.addr, align 16)

 declare void @llvm.var.annotation(i8*, i8*, i8*, i32)
 define void @test_var_annotation(i8*, i8*, i8*, i32) {
   ; CHECK-LABEL: name:            test_var_annotation
   ; CHECK-NOT: llvm.var.annotation
   ; CHECK: RET_ReallyLR
   call void @llvm.var.annotation(i8* %0, i8* %1, i8* %2, i32 %3)
   ret void
 }
+
+!0 = !{ i64 0, i64 2 }