On Tue, Aug 14, 2012 at 1:28 PM, John Criswell <[email protected]> wrote: > On 8/14/12 3:02 PM, Eli Friedman wrote: >> >> On Tue, Aug 14, 2012 at 12:52 PM, John Criswell <[email protected]> >> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
