[PATCH] fix for PR16393: miscompile with struct byval

[Jim Grosbach]

On Jul 8, 2013, at 1:47 PM, Jakob Stoklund Olesen <jolesen at apple.com> wrote:

> 
> On Jul 8, 2013, at 1:01 PM, Jim Grosbach <grosbach at apple.com> wrote:
> 
>> I haven’t followed closely enough to know all the recent details, but I don’t like the idea of having a property on MachineBasicBlock. I much prefer the pseudo-instruction to that.
> 
> Same here.
> 
>> My only hesitation about teaching PEI to do a CFG walk is that it’s adding a non-trivial bit of complexity, especially in the presence of crazy CFGs with computed-gotos and such in them. In theory, those shouldn’t interact with stack adjustments, but in practice?
> 
> By complexity, do you mean compile time or amount of PEI code required?

The latter. I think compile time will be fine either way. My simplistic view of PEI is that it just iterates over the instructions in a function, in layout order, replaces frame indices along the way and keeps track of some interesting facts while doing so like which CSRs are clobbered. Anything that moves the algorithm further afield from that makes me nervous.

> I am not really concerned about compile time. We’re already (at least) linear in the number of CFG edges, and this won’t add much to that term.

I thought PEI was constant on the CFG and linear on the number of basic blocks unless shrink-wrapping was going on. I confess I haven’t looked at the guts of the algorithm in a long time, though, so I could easily be wrong. The scavenger is the nasty bit, when it fires, but has the hard-coded cap on how far to scan to prevent things from going too badly. Anyways, I don’t see this as a significant concern for the problem at hand.

> As for code complexity, this will certainly make PEI more complex, but I think it is comparable to all the places that would need to learn how to maintain the SET_SP_ADJ invariants. I’d prefer to have the mess all in one place.

Entirely possible. My first thought is that there wouldn’t need to be many modifications elsewhere and the pseudo would just flow on through w/o getting mucked with. That may be overly optimistic, though.

>> The only bit I feel strongly about is that I agree we should have something that tries to verify those three invariants.
> 
> Definitely. It would probably be a good thing to add to the machine code verifier no matter what we decide to do with PEI.
> 

Yep.

-Jim
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130708/39ffa627/attachment.html>