[PATCH] D52785: [PseudoSourceValue] New category to represent floating-point status

[Matthias Braun via llvm-commits]

> On Oct 8, 2018, at 4:30 PM, Matthias Braun via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
>> 
>> On Oct 8, 2018, at 4:08 PM, Hal Finkel via Phabricator via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>> 
>> hfinkel added a comment.
>> 
>> In https://reviews.llvm.org/D52785#1258222, @MatzeB wrote:
>> 
>>> As far as I understand MachineMemoryOperands:
>>> 
>>> - You are allowed to drop them
>> 
>> 
>> I'm trying to understand the difference between the world where we're allowed to drop these and the world in which we're not. It's not clear what benefit we get from allowing dropping these. There also might be an advantage to dropping the Value*s used for AA (e.g., to better decouple the MI from the IR) which doesn't extend to MMOs in general.
>> 
>>> - An operation without MachineMemoryOperand needs to be handled conservatively (i.e. potentially aliases with everything)
>> 
>> This is certainly true, at least as far as aliasing goes. Regarding volatile semantics it doesn't work.
>> 
>> As I see it, we don't currently drop MMOs in practice (with the exception, now, of the MI outliner?). Doing so breaks, at least, volatile semantics. I think that we should simply document that MMOs can't be dropped in general (although we might drop parts of them, such as AA Value*). Thoughts?
> Looking at relevant uses to get a feeling for this:
> 
> - I think various places that "invent" memory operations are not in a good position to create meaningful MMOs so they keep the list empty.
> - I think the motivation in the outliner code is just that the IR references become invalid after cloning instructions from one function into another (simply because there is no code to also clone the IR as well in the outliner).
> - MachineInstr::cloneMergedMemRefs() logic which is used when merging basic blocks with common tails has some rules like: merging an empty MMO list with another results in an empty MMO list.
> 
> It's indeed mostly motivated by the Value* for AA.
> 
> However shouldn't we get volatile like semantics just by dropping the memory operands? Looking around the code it seems that MachineInstr::isSafeToMove() behaves that way, however I think ScheduleDAGInstrs does not behave the same for volatiles as for MMO-less instructions. I'm tempted to declare this a bug a ScheduleDAGInstrs...

I think the ScheduleDAGInstrs behavior accidentally changed in r315565 / https://reviews.llvm.org/D38735 <https://reviews.llvm.org/D38735>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181008/f892593c/attachment.html>