[llvm] r312318 - Debug info for variables whose type is shrinked to bool
Mikael Holmén via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 21 03:54:56 PDT 2017
Hi Nikola and reviewers,
If I understand correctly this change was resubmitted in r313870?
Did you consider the problem I described below? Is it solved/cannot
happen now?
Regards,
Mikael
On 09/07/2017 09:36 AM, Mikael Holmén wrote:
> Hi Nikola and reviewers,
>
> I think the debug info produced with this patch triggers a problem in llc.
>
> Specifically, in DwarfDebug::beginModule() we do
>
> CU.getOrCreateGlobalVariableDIE(GV, sortGlobalExprs(GVMap[GV]));
>
> It's the sorting done by sortGlobalExprs that doesn't seem to handle the
> case when we have mixed DW_OP_LLVM_fragment and DW_OP_deref very well.
>
> Input to sort:
> !DIExpression(DW_OP_LLVM_fragment, 0, 16)
> !DIExpression(DW_OP_LLVM_fragment, 16, 16)
> !DIExpression(DW_OP_LLVM_fragment, 32, 16)
> !DIExpression(DW_OP_LLVM_fragment, 48, 16)
> !DIExpression(DW_OP_deref, DW_OP_constu, 1, DW_OP_mul, DW_OP_constu, 0,
> DW_OP_plus, DW_OP_stack_value)
> !DIExpression(DW_OP_LLVM_fragment, 80, 16)
> !DIExpression(DW_OP_LLVM_fragment, 96, 16)
> !DIExpression(DW_OP_LLVM_fragment, 112, 16)
> !DIExpression(DW_OP_LLVM_fragment, 128, 16)
> !DIExpression(DW_OP_LLVM_fragment, 144, 16)
> !DIExpression(DW_OP_LLVM_fragment, 160, 16)
> !DIExpression(DW_OP_LLVM_fragment, 176, 16)
> !DIExpression(DW_OP_LLVM_fragment, 192, 16)
> !DIExpression(DW_OP_LLVM_fragment, 208, 16)
> !DIExpression(DW_OP_LLVM_fragment, 224, 16)
> !DIExpression(DW_OP_deref, DW_OP_constu, 1, DW_OP_mul, DW_OP_constu, 0,
> DW_OP_plus, DW_OP_stack_value)
> !DIExpression(DW_OP_LLVM_fragment, 256, 16)
>
> Output from sort:
> !DIExpression(DW_OP_LLVM_fragment, 0, 16)
> !DIExpression(DW_OP_LLVM_fragment, 16, 16)
> !DIExpression(DW_OP_LLVM_fragment, 32, 16)
> !DIExpression(DW_OP_LLVM_fragment, 48, 16)
> !DIExpression(DW_OP_LLVM_fragment, 128, 16)
> !DIExpression(DW_OP_deref, DW_OP_constu, 1, DW_OP_mul, DW_OP_constu, 0,
> DW_OP_plus, DW_OP_stack_value)
> !DIExpression(DW_OP_LLVM_fragment, 80, 16)
> !DIExpression(DW_OP_LLVM_fragment, 96, 16)
> !DIExpression(DW_OP_LLVM_fragment, 112, 16)
> !DIExpression(DW_OP_LLVM_fragment, 144, 16)
> !DIExpression(DW_OP_LLVM_fragment, 160, 16)
> !DIExpression(DW_OP_LLVM_fragment, 176, 16)
> !DIExpression(DW_OP_LLVM_fragment, 192, 16)
> !DIExpression(DW_OP_LLVM_fragment, 208, 16)
> !DIExpression(DW_OP_LLVM_fragment, 224, 16)
> !DIExpression(DW_OP_deref, DW_OP_constu, 1, DW_OP_mul, DW_OP_constu, 0,
> DW_OP_plus, DW_OP_stack_value)
> !DIExpression(DW_OP_LLVM_fragment, 256, 16)
>
> The above "sorting" then causes an assert to blow in
> DwarfExpression::addFragmentOffset since that function seems to assume
> that we handle fragments in strictly growing offset order.
>
> The sortGlobalExprs looks like
>
> /// Sort and unique GVEs by comparing their fragment offset.
> static SmallVectorImpl<DwarfCompileUnit::GlobalExpr> &
> sortGlobalExprs(SmallVectorImpl<DwarfCompileUnit::GlobalExpr> &GVEs) {
> std::sort(GVEs.begin(), GVEs.end(),
> [](DwarfCompileUnit::GlobalExpr A,
> DwarfCompileUnit::GlobalExpr B) {
> if (A.Expr != B.Expr && A.Expr && B.Expr) {
> auto FragmentA = A.Expr->getFragmentInfo();
> auto FragmentB = B.Expr->getFragmentInfo();
> if (FragmentA && FragmentB)
> return FragmentA->OffsetInBits <
> FragmentB->OffsetInBits;
> }
> return false;
> });
> GVEs.erase(std::unique(GVEs.begin(), GVEs.end(),
> [](DwarfCompileUnit::GlobalExpr A,
> DwarfCompileUnit::GlobalExpr B) {
> return A.Expr == B.Expr;
> }),
> GVEs.end());
> return GVEs;
> }
>
> if I add
>
> if (FragmentA)
> return true;
>
> after the check
>
> if (FragmentA && FragmentB)
> return FragmentA->OffsetInBits <
>
> then it gets nicely sorted.
>
> But maybe we need to be even more careful if we want to make sure the
> fragments are all properly sorted when there are all kinds of things in
> the input, and we should change the sorting to:
> // Sort all expressions containing fragment offsets (relative to each
> other),
> // and put all other expressions in random order (relative to each
> other) but
> // after the fragment expressions.
> std::sort(GVEs.begin(), GVEs.end(),
> [](DwarfCompileUnit::GlobalExpr A,
> DwarfCompileUnit::GlobalExpr B) {
> auto FragmentA = A.Expr ? A.Expr->getFragmentInfo() : None;
> auto FragmentB = B.Expr ? B.Expr->getFragmentInfo() : None;
> if (FragmentA && FragmentB)
> return FragmentA->OffsetInBits < FragmentB->OffsetInBits;
> if (FragmentA)
> return true;
> return false;
> });
>
> so we ensure a proper sorting also when there are GlobalExprs with no Expr?
>
> Also note that the attempt to remove duplicates in sortGlobalExprs will
> not remove all duplicates! Since std::unique only removes _consecutive_
> equal elements, and the aobve sorting really only sorts the fragments,
> it might leave duplicate non-fragment expressions. I've no idea if this
> is a problem though.
>
> I see the above problems with
>
> llc -o - sort.ll
>
> where sort.ll is a heavily reduced ll file originally produced using
> r312318. llc end stops with
>
> llc: ../lib/CodeGen/AsmPrinter/DwarfExpression.cpp:405: void
> llvm::DwarfExpression::addFragmentOffset(const llvm::DIExpression *):
> Assertion `FragmentOffset >= OffsetInBits && "overlapping or duplicate
> fragments"' failed.
>
> Also note that if I use another compiler to compile llc (e.g. gcc
> compared to clang) the problem assert doesn't show up. I suppose this is
> due to luck, and has to do with the order in which std::sort compares
> the objects in the input.
>
> Regards,
> Mikael
>
> On 09/01/2017 12:05 PM, Strahinja Petrovic via llvm-commits wrote:
>> Author: spetrovic
>> Date: Fri Sep 1 03:05:27 2017
>> New Revision: 312318
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=312318&view=rev
>> Log:
>> Debug info for variables whose type is shrinked to bool
>>
>> This patch provides such debug information for integer
>> variables whose type is shrinked to bool by providing
>> dwarf expression which returns either constant initial
>> value or other value.
>>
>> Patch by Nikola Prica.
>>
>> Differential Revision: https://reviews.llvm.org/D35994
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
>> llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>> llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp?rev=312318&r1=312317&r2=312318&view=diff
>>
>> ==============================================================================
>>
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.cpp Fri Sep 1
>> 03:05:27 2017
>> @@ -338,6 +338,7 @@ void DwarfExpression::addExpression(DIEx
>> break;
>> case dwarf::DW_OP_plus:
>> case dwarf::DW_OP_minus:
>> + case dwarf::DW_OP_mul:
>> emitOp(Op->getOp());
>> break;
>> case dwarf::DW_OP_deref:
>>
>> Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfoMetadata.cpp?rev=312318&r1=312317&r2=312318&view=diff
>>
>> ==============================================================================
>>
>> --- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
>> +++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Fri Sep 1 03:05:27 2017
>> @@ -643,6 +643,7 @@ bool DIExpression::isValid() const {
>> case dwarf::DW_OP_plus_uconst:
>> case dwarf::DW_OP_plus:
>> case dwarf::DW_OP_minus:
>> + case dwarf::DW_OP_mul:
>> case dwarf::DW_OP_deref:
>> case dwarf::DW_OP_xderef:
>> break;
>>
>> Modified: llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp?rev=312318&r1=312317&r2=312318&view=diff
>>
>> ==============================================================================
>>
>> --- llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp (original)
>> +++ llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp Fri Sep 1 03:05:27 2017
>> @@ -36,6 +36,7 @@
>> #include "llvm/IR/Module.h"
>> #include "llvm/IR/Operator.h"
>> #include "llvm/IR/ValueHandle.h"
>> +#include "llvm/IR/DebugInfoMetadata.h"
>> #include "llvm/Pass.h"
>> #include "llvm/Support/Debug.h"
>> #include "llvm/Support/ErrorHandling.h"
>> @@ -1603,12 +1604,47 @@ static bool TryToShrinkGlobalToBoolean(G
>> assert(InitVal->getType() != Type::getInt1Ty(GV->getContext()) &&
>> "No reason to shrink to bool!");
>> + SmallVector<DIGlobalVariableExpression *, 1> GVs;
>> + GV->getDebugInfo(GVs);
>> +
>> // If initialized to zero and storing one into the global, we can
>> use a cast
>> // instead of a select to synthesize the desired value.
>> bool IsOneZero = false;
>> - if (ConstantInt *CI = dyn_cast<ConstantInt>(OtherVal))
>> + if (ConstantInt *CI = dyn_cast<ConstantInt>(OtherVal)){
>> IsOneZero = InitVal->isNullValue() && CI->isOne();
>> + ConstantInt *CIInit = dyn_cast<ConstantInt>(GV->getInitializer());
>> + uint64_t ValInit = CIInit->getZExtValue();
>> + uint64_t ValOther = CI->getZExtValue();
>> + uint64_t ValMinus = ValOther - ValInit;
>> +
>> + for(auto *GVe : GVs){
>> + DIGlobalVariable *DGV = GVe->getVariable();
>> + DIExpression *E = GVe->getExpression();
>> +
>> + // val * (ValOther - ValInit) + ValInit:
>> + // DW_OP_deref DW_OP_constu <ValMinus>
>> + // DW_OP_mul DW_OP_constu <ValInit> DW_OP_plus DW_OP_stack_value
>> + E = DIExpression::get(NewGV->getContext(),
>> + {dwarf::DW_OP_deref,
>> + dwarf::DW_OP_constu,
>> + ValMinus,
>> + dwarf::DW_OP_mul,
>> + dwarf::DW_OP_constu,
>> + ValInit,
>> + dwarf::DW_OP_plus,
>> + dwarf::DW_OP_stack_value});
>> + DIGlobalVariableExpression *DGVE =
>> + DIGlobalVariableExpression::get(NewGV->getContext(), DGV, E);
>> + NewGV->addDebugInfo(DGVE);
>> + }
>> + } else {
>> + // FIXME: This will only emit address for debugger on which will
>> + // be written only 0 or 1.
>> + for(auto *GV : GVs)
>> + NewGV->addDebugInfo(GV);
>> + }
>> +
>> while (!GV->use_empty()) {
>> Instruction *UI = cast<Instruction>(GV->user_back());
>> if (StoreInst *SI = dyn_cast<StoreInst>(UI)) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
More information about the llvm-commits
mailing list