[llvm] r176022 - Fix the root cause of PR15348 by correctly handling alignment 0 on

[Lang Hames]

Hey,

I don't know either - I was quoting your analysis from the commit message.
;)

To be clear - I wasn't proposing a change to the definition of the IR,
though that has crossed on my mind too. I'm saying that I think the DAGs
actually rely on the alignment being non-zero. Your fix should make sure
that it is, but to be safe I'd like to add asserts at the point where the
alignment is actually used.

Cheers,
Lang.

On Thu, Mar 28, 2013 at 12:10 AM, Chandler Carruth <chandlerc at gmail.com>wrote:

> On Wed, Mar 27, 2013 at 11:52 PM, Lang Hames <lhames at gmail.com> wrote:
>
>> Hi Chandler, Dan,
>>
>>
>>> When alignment is zero, the lang ref says that *no* alignment
>>> assumptions can be made. This is the exact opposite of the internal API
>>> contracts of the DAG where alignment 0 indicates that the alignment can
>>> be made to be anything desired.
>>>
>>
>> I had occasion to circle back to this recently, and I wanted to
>> double-check this part of the analysis. Is this actually an internal
>> contract, and expected behavior? Or do things just go haywire when we feed
>> in a zero alignment? On a local LLVM copy, I replaced your fix with an
>> assert on zero-alignment, and added similar asserts to
>> getMemcpyLoadsAndStores (and friends), but wasn't able to trigger them with
>> anything in the test suite.
>>
>> If it's not an internal contract, but just the DAG going haywire on
>> Align==0, I'd like to keep your fix in place, and commit the asserts that
>> I've added to getMemcpyLoadsAndStores and friends.
>>
>
> My fix definitely was that the DAG went haywire on align == 0.
>
> I'm not sure what you mean by 'internal contract'. The LangRef sets out
> the meaning of align==0 for memset and friends very explicitly. I'm OK if
> you want to change the langref to say something else, but it's not about
> adding asserts, it's about changing the langref, adding autoupgrade
> functionality, and teaching the verifier about this first.... then you can
> add asserts. =D
>
>
>> Cheers,
>> Lang.
>>
>>
>> _______________________________________________
>> 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/20130328/e877ddf5/attachment.html>