[PATCH] D56587: Fix sign/zero extension in Dwarf expressions.
Markus Lavin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 11 04:05:00 PST 2019
markus created this revision.
markus added reviewers: aprantl, vsk.
markus added a project: debug-info.
This patch addresses two issues in llvm::replaceAllDbgUsesWith
i) Implementation of sign extension is broken and does not compute sext properly.
ii) It is probably not safe to assume that the consumer/debugger would set the high bits to zero in zero extension in e.g. the case that the variable has been spilled to memory.
Repository:
rL LLVM
https://reviews.llvm.org/D56587
Files:
lib/Transforms/Utils/Local.cpp
test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
unittests/Transforms/Utils/LocalTest.cpp
Index: unittests/Transforms/Utils/LocalTest.cpp
===================================================================
--- unittests/Transforms/Utils/LocalTest.cpp
+++ unittests/Transforms/Utils/LocalTest.cpp
@@ -790,29 +790,34 @@
// Case 1: The original expr is empty, so no deref is needed.
EXPECT_TRUE(hasADbgVal({DW_OP_dup, DW_OP_constu, 31, DW_OP_shr, DW_OP_lit0,
- DW_OP_not, DW_OP_mul, DW_OP_or, DW_OP_stack_value}));
+ DW_OP_not, DW_OP_mul, DW_OP_constu, 32, DW_OP_shl,
+ DW_OP_or, DW_OP_stack_value}));
// Case 2: Perform an address calculation with the original expr, deref it,
// then sign-extend the result.
EXPECT_TRUE(hasADbgVal({DW_OP_lit0, DW_OP_mul, DW_OP_deref, DW_OP_dup,
DW_OP_constu, 31, DW_OP_shr, DW_OP_lit0, DW_OP_not,
- DW_OP_mul, DW_OP_or, DW_OP_stack_value}));
+ DW_OP_mul, DW_OP_constu, 32, DW_OP_shl,
+ DW_OP_or, DW_OP_stack_value}));
// Case 3: Insert the sign-extension logic before the DW_OP_stack_value.
EXPECT_TRUE(hasADbgVal({DW_OP_lit0, DW_OP_mul, DW_OP_dup, DW_OP_constu, 31,
- DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_or,
+ DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul,
+ DW_OP_constu, 32, DW_OP_shl, DW_OP_or,
DW_OP_stack_value}));
// Cases 4-6: Just like cases 1-3, but preserve the fragment at the end.
EXPECT_TRUE(hasADbgVal({DW_OP_dup, DW_OP_constu, 31, DW_OP_shr, DW_OP_lit0,
- DW_OP_not, DW_OP_mul, DW_OP_or, DW_OP_stack_value,
- DW_OP_LLVM_fragment, 0, 8}));
+ DW_OP_not, DW_OP_mul, DW_OP_constu, 32, DW_OP_shl,
+ DW_OP_or, DW_OP_stack_value, DW_OP_LLVM_fragment, 0, 8}));
EXPECT_TRUE(
hasADbgVal({DW_OP_lit0, DW_OP_mul, DW_OP_deref, DW_OP_dup, DW_OP_constu,
- 31, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_or,
+ 31, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul,
+ DW_OP_constu, 32, DW_OP_shl, DW_OP_or,
DW_OP_stack_value, DW_OP_LLVM_fragment, 0, 8}));
EXPECT_TRUE(hasADbgVal({DW_OP_lit0, DW_OP_mul, DW_OP_dup, DW_OP_constu, 31,
- DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_or,
+ DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul,
+ DW_OP_constu, 32, DW_OP_shl, DW_OP_or,
DW_OP_stack_value, DW_OP_LLVM_fragment, 0, 8}));
verifyModule(*M, &errs(), &BrokenDebugInfo);
Index: test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
===================================================================
--- test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
+++ test/Transforms/InstCombine/cast-set-preserve-signed-dbg-val.ll
@@ -14,7 +14,7 @@
;
; The high 16 bits of the original 'and' require sign-extending the new 16-bit and:
; CHECK-NEXT: call void @llvm.dbg.value(metadata i16 [[and]], metadata [[C:![0-9]+]],
- ; CHECK-SAME: metadata !DIExpression(DW_OP_dup, DW_OP_constu, 15, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_or, DW_OP_stack_value)
+ ; CHECK-SAME: metadata !DIExpression(DW_OP_dup, DW_OP_constu, 15, DW_OP_shr, DW_OP_lit0, DW_OP_not, DW_OP_mul, DW_OP_constu, 16, DW_OP_shl, DW_OP_or, DW_OP_stack_value)
%D = trunc i32 %C to i16, !dbg !42
call void @llvm.dbg.value(metadata i16 %D, metadata !38, metadata !DIExpression()), !dbg !42
Index: lib/Transforms/Utils/Local.cpp
===================================================================
--- lib/Transforms/Utils/Local.cpp
+++ lib/Transforms/Utils/Local.cpp
@@ -1850,18 +1850,25 @@
bool Signed = *Signedness == DIBasicType::Signedness::Signed;
- if (!Signed) {
- // In the unsigned case, assume that a debugger will initialize the
- // high bits to 0 and do a no-op conversion.
- return Identity(DII);
+ if (Signed) {
+ // Sign extend To to FromBits
+ // (((To >> (ToBits - 1)) * (~0)) << ToBits) | To
+ SmallVector<uint64_t, 11> Ops({dwarf::DW_OP_dup,
+ dwarf::DW_OP_constu, ToBits - 1,
+ dwarf::DW_OP_shr,
+ dwarf::DW_OP_lit0, dwarf::DW_OP_not,
+ dwarf::DW_OP_mul,
+ dwarf::DW_OP_constu, ToBits,
+ dwarf::DW_OP_shl,
+ dwarf::DW_OP_or});
+ return DIExpression::appendToStack(DII.getExpression(), Ops);
} else {
- // In the signed case, the high bits are given by sign extension, i.e:
- // (To >> (ToBits - 1)) * ((2 ^ FromBits) - 1)
- // Calculate the high bits and OR them together with the low bits.
- SmallVector<uint64_t, 8> Ops({dwarf::DW_OP_dup, dwarf::DW_OP_constu,
- (ToBits - 1), dwarf::DW_OP_shr,
- dwarf::DW_OP_lit0, dwarf::DW_OP_not,
- dwarf::DW_OP_mul, dwarf::DW_OP_or});
+ // Zero extend To to FromBits i.e. make sure that any bits in the range
+ // [FromBits-1:ToBits] are cleared.
+ // (To & ((1 << ToBits) - 1))
+ SmallVector<uint64_t, 3> Ops({dwarf::DW_OP_constu,
+ (1ULL << ToBits) - 1,
+ dwarf::DW_OP_and});
return DIExpression::appendToStack(DII.getExpression(), Ops);
}
};
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56587.181241.patch
Type: text/x-patch
Size: 5784 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190111/7f291151/attachment.bin>
More information about the llvm-commits
mailing list