[PATCH] D31062: PR32288: Describe a bool parameter's DWARF location with a simple register

[Adrian Prantl via llvm-commits]

> On Mar 17, 2017, at 11:34 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> I suppose the question is: is it worth/necessary to provide a test case? We could include a fixme & not do anything here? Or I can work up an IR test case for IR Clang (& probably Swift) don't currently generate, so behavior that won't be used.

IMO if the IR passes the Verifier, we should support it. And we didn't test other frontends, such as Julia, for example. If it is just a matter of making a small modification to IR generated by clang, I feel more comfortable with supporting it.

-- adrian

> 
> On Fri, Mar 17, 2017 at 11:15 AM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
>> On Mar 17, 2017, at 9:12 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> Hmm, one wrinkle... 
>> 
>> the "if (auto Fragment ...)" case doesn't run in any existing test cases. (I put an "assert(false)" there & ran check-llvm).
>> 
>> Any ideas if that's still reachable? Perhaps this only happens for direct bool parameters, never fragments.
> 
> I just finished building the swift stdlib in 64 and 32 bit (which is typically a good source of these) and also didn't hit the assertion. We'd need to hand-craft a testcase for opt -mem2reg.
> 
> -- adrian
> 
>> 
>> On Fri, Mar 17, 2017 at 8:28 AM Adrian Prantl via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
>> aprantl accepted this revision.
>> aprantl added a comment.
>> This revision is now accepted and ready to land.
>> 
>> Took me a while to find the functional change amidst the refactoring :-)
>> As discussed in the PR, this LGTM, thanks!
>> One inline comment about a comment.
>> 
>> 
>> 
>> ================
>> Comment at: lib/Transforms/Utils/Local.cpp:1118
>>      // describing will always be smaller than the variable size, because
>>      // VariableSize == Size of Alloca described by DDI. Since SI stores
>>      // to the alloca described by DDI, if it's first operand is an extend,
>> ----------------
>> Does this comment still match our understanding?
>> Particularly the `The fragment we're describing will always be smaller than the variable size` bit.
>> 
>> 
>> https://reviews.llvm.org/D31062 <https://reviews.llvm.org/D31062>
>> 
>> 
>> 

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