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

[Heejin Ahn via llvm-commits]

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/20180416/09d1c8f4/attachment-0001.html>