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

[David Blaikie via llvm-commits]

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/bee01ec7/attachment.html>