[cfe-commits] Patch for Bug 13606

[Eli Friedman]

On Tue, Aug 14, 2012 at 1:28 PM, John Criswell <criswell at illinois.edu> wrote:
> On 8/14/12 3:02 PM, Eli Friedman wrote:
>>
>> On Tue, Aug 14, 2012 at 12:52 PM, John Criswell <criswell at illinois.edu>
>> wrote:
>>>
>>> Dear All,
>>>
>>> Attached is a patch to partially fix the alignment attribute bug in
>>> PR#13606:
>>>
>>> http://llvm.org/bugs/show_bug.cgi?id=13606
>>>
>>> I say partially because the alignment should really be unsigned, but
>>> since
>>> CharUnits uses a signed value, it isn't clear to me how to fix the bug
>>> properly (i.e., it requires a greater knowledge of clang than I current
>>> have
>>> time to learn).  That said, permitting alignment up to half the address
>>> space should be a significant improvement.
>>>
>>> If this patch meets your approval, please let me know, and I can commit
>>> it
>>> (I already have commit access to the LLVM SVN repository).
>>
>> Please don't use "long"; use int64_t.  "long" doesn't have a
>> consistent size across the platforms we support.
>
>
> Okay, I fixed that.
>
>>
>> Please include a testcase.
>
>
> Done, although should it have a date in the name, or is the current name
> okay?

The current name is fine.  I was sort of hoping for a testcase with a
RUN-line more like "clang -emit-llvm -o - %s | FileCheck %s", though.
(i.e. check that we emit the correct alignment into the IR.)

> Regarding the bug report, should I close it, or should I leave it open since
> it's possible to have problems with alignments greater than half the address
> space?

I would say just close it; nobody is likely to use alignments that large.

-Eli