[llvm] r330079 - [WebAssembly] Fix a bug in MachineBasicBlock::findDebugLoc() call

[Heejin Ahn via llvm-commits]

Not sure what kind of assertion you are talking about. The invalid IR I was
testing with cannot be generated in the current compiler.

Oh, and I was not very precise in the previous email: that kind of IR is
not gonna be valid IR; that kind of IR will be prevented anyway after
adding EH support, because BB sorting routine will be taking EH CFG into
consideration.

On Mon, Apr 16, 2018 at 4:53 PM David Blaikie <dblaikie at gmail.com> wrote:

> Assertion until it can be tested, perhaps? Not sure.
>
> On Mon, Apr 16, 2018 at 4:51 PM Heejin Ahn <aheejin at gmail.com> wrote:
>
>> It's soon to be valid IR actually. I'm adding exception handling support
>> to wasm now and that kind of IR can be valid soon (within a couple weeks).
>> But I can't add it right now because the EH support is not there. But
>> without this patch I couldn't even land any other patches.
>>
>> On Mon, Apr 16, 2018 at 3:54 PM David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> If it's untestable and only impacts invalid IR, maybe an assertion would
>>> be better? (& then your invalid IR would fail sooner)
>>>
>>> On Mon, Apr 16, 2018 at 12:15 PM Heejin Ahn <aheejin at gmail.com> wrote:
>>>
>>>> It's hard to provide a test case that makes sense, because the failing
>>>> condition only happens when a basic block does not have a terminator but
>>>> branches to another BB that is not a layout successor (= a BB that comes
>>>> right after the current BB), which is not possible. I accidentally found
>>>> that bug while debugging my buggy and not-well-formed IR, which cannot be
>>>> generated from a valid ll file. The fix was very trivial, almost to the
>>>> extent of a typo fix, and the reviewer of that patch (yurydelendik@)
>>>> was who wrote that line of the code a month ago, so I thought he could make
>>>> sure it was correct. I CC'ed him here.
>>>>
>>>> On Mon, Apr 16, 2018 at 9:41 AM David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>> Test case?
>>>>>
>>>>> On Fri, Apr 13, 2018 at 5:15 PM Heejin Ahn via llvm-commits <
>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>
>>>>>> Author: aheejin
>>>>>> Date: Fri Apr 13 17:12:12 2018
>>>>>> New Revision: 330079
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=330079&view=rev
>>>>>> Log:
>>>>>> [WebAssembly] Fix a bug in MachineBasicBlock::findDebugLoc() call
>>>>>>
>>>>>> Summary:
>>>>>> InsertPos is within the bacic block `Header`, so `findDebugLoc()`
>>>>>> should
>>>>>> be called on not `MBB` but `Header` instead.
>>>>>>
>>>>>> Reviewers: yurydelendik
>>>>>>
>>>>>> Subscribers: jfb, dschuff, aprantl, sbc100, jgravelle-google,
>>>>>> sunfish, JDevlieghere, llvm-commits
>>>>>>
>>>>>> Differential Revision: https://reviews.llvm.org/D45648
>>>>>>
>>>>>> Modified:
>>>>>>     llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
>>>>>>
>>>>>> Modified: llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp?rev=330079&r1=330078&r2=330079&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp
>>>>>> (original)
>>>>>> +++ llvm/trunk/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp Fri
>>>>>> Apr 13 17:12:12 2018
>>>>>> @@ -147,9 +147,10 @@ static void PlaceBlockMarker(
>>>>>>    }
>>>>>>
>>>>>>    // Add the BLOCK.
>>>>>> -  MachineInstr *Begin = BuildMI(*Header, InsertPos,
>>>>>> MBB.findDebugLoc(InsertPos),
>>>>>> -                                TII.get(WebAssembly::BLOCK))
>>>>>> -
>>>>>> .addImm(int64_t(WebAssembly::ExprType::Void));
>>>>>> +  MachineInstr *Begin =
>>>>>> +      BuildMI(*Header, InsertPos, Header->findDebugLoc(InsertPos),
>>>>>> +              TII.get(WebAssembly::BLOCK))
>>>>>> +          .addImm(int64_t(WebAssembly::ExprType::Void));
>>>>>>
>>>>>>    // Mark the end of the block.
>>>>>>    InsertPos = MBB.begin();
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at lists.llvm.org
>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180417/b959e78e/attachment.html>