[llvm] r209007 - Fix most of PR10367.

John McCall rjmccall at apple.com
Sun May 18 14:30:33 PDT 2014


On May 18, 2014, at 7:31 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>> We strongly disagree on this design, and that’s fine.  But I do not see why it matters, because you not only haven’t proposed it, but you haven’t even fleshed it out to yourself yet, which is, again, fine as far as your proposal for ConstantExpr goes.  The problem is that you and Rafael have decided to go ahead and use the basic thrust of this less-than-half-baked proposal to completely redesign an existing LLVM feature in a way that intentionally constrains and complicates future extensions.  That’s using trunk as an experimental lab for IR design, and it seems totally inappropriate to me.
> 
> Sorry, what?
> 
> I removed a "feature" that would not survive llvm-as + llvm-dis. I did
> so following a design that was documented in bugzilla and discussed in
> the dev meeting.  I explicitly talked with both Chris and Nick Kledzik
> to make sure I would understand where we wanted go.

This is all good, but none of them are the accepted process for proposing
a design change to IR, which is to send a detailed proposal to the
discussion list and wait a reasonable amount of time for consensus to
develop.  Patch review is not an adequate substitute.

> I committed the patch after having it reviewed by a Google and an
> Apple employee.

I’m sorry that we didn’t catch it in patch review anyway; that’s our fault.

> This is the first time I see any objection to it and in summary it
> sounds like: I may some day want to change this is some unspecified
> way.

We have patches, that we aren’t yet certain are ready for trunk, that allow
GEPs in global aliases, which I think is a very clean way to expressed
offsetted aliases.  Those patches are now basically impossible to apply
because you’ve deliberately neutered the representation of global aliases.
Now, obviously, you don’t have a responsibility to allow our out-of-tree
patches to apply cleanly, and I also know that you’re working on
adding offsets, which will address our immediate need.  Nonetheless, I’m
concerned that you’re taking global aliases in a needlessly restrictive
direction.

John.



More information about the llvm-commits mailing list