On 11/16/2015 05:07 PM, Alexandre Oliva wrote:
The check is not in my patch, indeed. That's because the previous block
performs the runtime check, and it only lets through two cases: the one
we handle, and the one nobody uses.
That was the conclusion I was starting to come to, but expressed so
poorly in my last message. Sadly it was non-obvious from staring at the
current code. Though I must admit that after a week, I can see it
better now. Maybe that's a result of re-reading your message a
half-dozen more times with the current code and your patch all visible
in windows next to each other :-)
Prior to your change we'd just blindly copy from ENTRY_PARM to MEM,
which would result in missing the implicit shift if MEM wasn't actually
a memory.
You're just moving that conditional up and handling the shift
explicitly. You've got asserts for the cases you're not handling (and
no, I'm not aware of the need for this on any LE architecture, while I
am aware of BE architectures that align in both directions).
Any suggestions on how to improve the comments so that they convey
enough of this reasoning to make sense, without our having to write a
book :-) on the topic?
Refer back to this thread? :-) Seriously though, looking at things a
week later, I can see it much better now. Thanks for your patience on this.
OK for the trunk,
jeff
