[PATCH] Make DataLayout Non-Optional in the Module

[Eric Christopher]

On Wed, Mar 4, 2015 at 1:20 PM Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> > On Mar 4, 2015, at 12:20 PM, Eric Christopher <echristo at gmail.com>
> wrote:
> >
> > Couple of inline comments. Otherwise I think this looks fine to me.
> >
> > -eric
> >
> >
> > ================
> > Comment at: test/Transforms/InstCombine/load-cmp.ll:1
> > @@ -1,3 +1,2 @@
> > -; RUN: opt -instcombine -S < %s | FileCheck -check-prefix=NODL %s
> > -; RUN: opt -instcombine -S -default-data-layout="p:32:32:32-p1:16:16:16-n8:16:32:64"
> < %s | FileCheck -check-prefix=P32 %s
> > +; RUN: opt -instcombine -S -default-data-layout="p:32:32:32-p1:16:16:16-n8:16:32:64"
> < %s | FileCheck %s
> >
> > ----------------
> > This is the one I meant about default-data-layout, but I see what you
> mean. No reason to put it in the file other than being able to remove the
> option, but that's not necessary so OK.
>
> I’m not sure about the real value of the default-data-layout command line
> option, so I’m not against remove the option entirely.
> But I just didn’t care and assumed that the the option still make sense,
> and since we have an option I thought we better have a test for it.
>
>
I'm ambivalent myself :)


>
> >>
> >> So I used my lldb skills to find out, and with a DataLayout the alloca
> is "align 4", and the first store visited is in turn "aligned". It cannot
> be merge with the other one at this point since they are not identical. It
> is only when the second store is visited and "aligned" that they can be
> merged together. Hence the diff.
> >> Note: we could sort deterministically the PHI arguments using something
> else than "the order the store are visited", for instance using the
> relative ordering of the basic blocks. I'm not sure it matters that much as
> the order is still deterministic right now.
> > Again with a yuck, might be worth filing a bug or something on this.
>
> What exactly bugs you here?
> I mean, I’m not sure what to put in the bug report?
>
>
Hrm. I misread it as not deterministic. Now having things dependent upon
ordering in the phi is still pretty terrible, but not worse than anything
else we've got.

-eric


> Thanks,
>
> —
> Mehdi
>
>
> _______________________________________________
> 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/20150304/8f424919/attachment.html>