[llvm] r185966 - Reapply an improved version of r180816/180817.

[Quentin Colombet]

Adrian,

If you think you will need more time to fix the problem, please revert your patch.

Thanks,

-Quentin

On Jul 9, 2013, at 4:18 PM, Adrian Prantl <aprantl at apple.com> wrote:

> I'm on it, thanks!
> 
> On Jul 9, 2013, at 4:13 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Hi Adrian,
>> 
>> Your commit breaks a buildbot:
>> http://lab.llvm.org:8013/builders/clang-x86_64-darwin11-nobootstrap-RAincremental/builds/4150/
>> ******************** TEST 'Clang :: debuginfo-tests/aggregate-indirect-arg.cpp' FAILED ********************
>> Script:
>> --
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/Release+Asserts/bin/clang -ccc-cxx  -O0 -g /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/debuginfo-tests/aggregate-indirect-arg.cpp -c -o /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.tmp.o
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/Release+Asserts/bin/clang -ccc-cxx /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.tmp.o -o /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.tmp.out
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/llvm/utils/test_debuginfo.pl /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/debuginfo-tests/aggregate-indirect-arg.cpp /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.tmp.out
>> --
>> Exit Code: 1
>> Command Output (stderr):
>> --
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/debuginfo-tests/aggregate-indirect-arg.cpp:8:11: error: expected string not found in input
>> // CHECK: $1 = {
>>          ^
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.gdb.output:1:1: note: scanning from here
>> Reading symbols for shared libraries ... done
>> ^
>> /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang-build/tools/clang/test/debuginfo-tests/Output/aggregate-indirect-arg.cpp.gdb.output:2:9: note: possible intended match here
>> Breakpoint 1 at 0x100000d5c: file /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin11-nobootstrap-RAincremental/clang.src/test/debuginfo-tests/aggregate-indirect-arg.cpp, line 22.
>>        ^
>> --
>> 
>> Could you have a look please?
>> 
>> Thanks,
>> 
>> -Quentin
>> 
>> On Jul 9, 2013, at 3:06 PM, Eric Christopher <echristo at gmail.com> wrote:
>> 
>>> On Tue, Jul 9, 2013 at 2:53 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>> Hey Eric,
>>>> 
>>>> thanks for the comments!
>>>> 
>>>> 
>>>> On Jul 9, 2013, at 2:15 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>> 
>>>>>> bool LDVImpl::handleDebugValue(MachineInstr *MI, SlotIndex Idx) {
>>>>>> // DBG_VALUE loc, offset, variable
>>>>>> if (MI->getNumOperands() != 3 ||
>>>>>> -      !MI->getOperand(1).isImm() || !MI->getOperand(2).isMetadata()) {
>>>>>> +      !(MI->getOperand(1).isReg() || MI->getOperand(1).isImm()) ||
>>>>>> +      !MI->getOperand(2).isMetadata()) {
>>>>> 
>>>>> Mind commenting this a bit more on what we aren't handling?
>>>> 
>>>> I’m not sure if I can come up with a comprehensive list of cases that are not handled. One case that I can think of would be a FrameIndex DBG_VALUE, but I’m not even sure whether they can even be introduced before LDV is run.
>>>> 
>>> 
>>> Yeah, I'm not positive either. It just seemed odd like it's checking
>>> for the validity of the instruction at all instead of some case that
>>> it's not handling.
>>> 
>>>> 
>>>>>> -  FuncInfo.ArgDbgValues.push_back(
>>>>>> +    FuncInfo.ArgDbgValues.push_back(BuildMI(MF, getCurDebugLoc(),
>>>>>> +                                            TII->get(TargetOpcode::DBG_VALUE),
>>>>>> +                                            /* IsIndirect */ Offset != 0,
>>>>>> +                                            Op->getReg(), Offset, Variable));
>>>>>> 
>>>>> 
>>>>> Curious about the "Offset != 0" since we could have a 0 offset in the
>>>>> dbg.value intrinsic coming in yes?
>>>> 
>>>> I agree! I’m gradually pushing the problem up layer by layer. Currently this is just a more explicit notation of the status quo. I will add in a comment.
>>>> 
>>> 
>>> Thanks :)
>>> 
>>> -eric
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130709/eb0f4db2/attachment.html>