[GlobalISel] Handle IntToPtr constants in dbg.value

[fdeazeve]

Summary

Currently, the IR to MIR translator can only handle two kinds of constant
inputs to dbg.values intrinsics: constant integers and constant floats. In
particular, it cannot handle pointers created from IntToPtr ConstantExpression
objects.

This patch addresses the limitation above by replacing the IntToPtr with
its input integer prior to converting the dbg.value input.

[fdeazeve]

Fixed typo in commit message

[aprantl]

Thanks! LGTM with the one inline comment addressed.

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

112

1. I don't think this code path is tested?
2. Is this inttoptr(float) actually legal LLVM IR?

[fdeazeve]

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

112

Is this inttoptr(float) actually legal LLVM IR?

No, this kind of IR would fail the verifier

I don't think this code path is tested?

I see the source of confusion now. I only changed this line to keep the spirit of the "type switch" that is going on here, i.e. the same value is tested on every branch.
For example, if the lambda ever changed to strip other things (for example some new float to ptr cast instruction), then the rest of the code wouldn't need to change.

Thoughts? I can argue both ways

[aprantl]

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

112

Ah okay, that makes sense!

[fdeazeve]

All tests failures are due to this linke error: "/usr/bin/ld: cannot find -lomptarget.devicertl: No such file or directory'
Probably some unrelated issue with lib OpenMP

[aprantl]

In D130642#3682886, @fdeazeve wrote:

All tests failures are due to this linke error: "/usr/bin/ld: cannot find -lomptarget.devicertl: No such file or directory'
Probably some unrelated issue with lib OpenMP

Yeah there's no way that is related to this patch.

[nikic]

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

103

What's the cast<const Constant>() for?

[fdeazeve]

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

103

This was done to drive the lambda return type deduction to const Constant*, which is the type of &C.

I thought getOperand would return a Value, but now that you mentioned it, I see that I was wrong, it already returns a Constant.

So I think this code could have been rewritten with an explicit return type of the lambda and the cast removed?

cpp
auto *NumericConstant = [&] () -> const Constant* {
    if (const auto *CE = dyn_cast<ConstantExpr>(&C))
      if (CE->getOpcode() == Instruction::IntToPtr)
        return CE->getOperand(0);
    return &C;
  }();

llvm/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp

MachineInstrBuilder MachineIRBuilder::buildConstDbgValue(const Constant &C,
                                                          const MDNode *Variable,
                                                          const MDNode *Expr) {
   assert(isa<DILocalVariable>(Variable) && "not a variable");
   assert(cast<DIExpression>(Expr)->isValid() && "not an expression");
   assert(
       cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(getDL()) &&
       "Expected inlined-at fields to agree");
   auto MIB = buildInstrNoInsert(TargetOpcode::DBG_VALUE);
-  if (auto *CI = dyn_cast<ConstantInt>(&C)) {
+
+  auto *NumericConstant = [&] {
+    if (const auto *CE = dyn_cast<ConstantExpr>(&C))
+      if (CE->getOpcode() == Instruction::IntToPtr)
+        return cast<const Constant>(CE->getOperand(0));
+    return &C;
+  }();
+
+  if (auto *CI = dyn_cast<ConstantInt>(NumericConstant)) {
     if (CI->getBitWidth() > 64)
       MIB.addCImm(CI);
     else
       MIB.addImm(CI->getZExtValue());
-  } else if (auto *CFP = dyn_cast<ConstantFP>(&C)) {
+  } else if (auto *CFP = dyn_cast<ConstantFP>(NumericConstant)) {
     MIB.addFPImm(CFP);
   } else {
     // Insert $noreg if we didn't find a usable constant and had to drop it.
     MIB.addReg(Register());
   }
 
   MIB.addImm(0).addMetadata(Variable).addMetadata(Expr);
   return insertInstr(MIB);

llvm/test/CodeGen/AArch64/GlobalISel/debug-insts.ll

 ; CHECK: DBG_VALUE %1(p0), $noreg, !17, !DIExpression(DW_OP_deref), debug-location !18
   call void @llvm.dbg.value(metadata i32* %addr, i64 0, metadata !17, metadata !DIExpression(DW_OP_deref)), !dbg !18
 ; CHECK: DBG_VALUE 123, 0, !17, !DIExpression(), debug-location !18
   call void @llvm.dbg.value(metadata i32 123, i64 0, metadata !17, metadata !DIExpression()), !dbg !18
 ; CHECK: DBG_VALUE float 1.000000e+00, 0, !17, !DIExpression(), debug-location !18
   call void @llvm.dbg.value(metadata float 1.000000e+00, i64 0, metadata !17, metadata !DIExpression()), !dbg !18
 ; CHECK: DBG_VALUE $noreg, 0, !17, !DIExpression(), debug-location !18
   call void @llvm.dbg.value(metadata i32* null, i64 0, metadata !17, metadata !DIExpression()), !dbg !18
+; CHECK: DBG_VALUE 42, 0, !17, !DIExpression(), debug-location !18
+  call void @llvm.dbg.value(metadata i32* inttoptr (i64 42 to i32*), i64 0, metadata !17, metadata !DIExpression()), !dbg !18
   ret void
 }
 
 ; Function Attrs: nounwind readnone
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 declare void @llvm.dbg.value(metadata, i64, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}