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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In https://reviews.llvm.org/D52785#1255051, @uweigand wrote:

> In https://reviews.llvm.org/D52785#1253328, @arsenm wrote:
>
> > How is this going to work? I don't think this can work unless we can start guaranteeing that MMOs are never dropped. Most FP instructions aren't considered as mayLoad or mayStore, and will probably easily lose the MMO if it's the only thing indicating they care about the FP mode
>
>
> I haven't seen MMOs being dropped with my current patchset.  I believe this should work because:
>
> - On the SelectionDAG level, we still have the STRICT_... nodes, which mostly aren't touched by optimizers anyway, and when we add some simplification for those nodes, they'll simply have to take care to preserve MMOs. As a sanity check, we could add an assert to verify that at the end of the DAG passes, every STRICT_... node still has a FPStatus MMO.
> - During selection of MIs from DAG nodes, the mayAccessMemory flag *does* help (in fact, this is why I introduced it in the first place ... early versions without that flag did indeed sometimes lose the MMO).  This is because of the change to SelectionDAGISel::SelectCodeCommon, which causes instruction patterns marked as mayAccessMemory to inherit MMOs from the selected DAG nodes.
> - On the MI level, once the instruction has the MMO, the MI.mayLoad() and/or MI.mayStore() routines will in fact return true (a mayAccessMemory instruction with MMO is considered equivalent to mayLoad/mayStore).  As far as I can see, this should cause the MI-level CodeGen passes to do the right thing (and preserve MMOs).
>
>   Am I missing anything here?  Where do you think MMOs might still get dropped?


I'm not sure where it's documented, but MMOs are definitely not guaranteed to be preserved (except in global-isel, the verifier enforces this for some of the G_* instructions). I'm not aware of anywhere specifically dropping them, but code is not required to preserve them. Instructions possibly accessing memory are then treated as conservatively as possible if they are missing. That doesn't work with this approach. The operand introduces stricter behavior. If something drops it, it now may be incorrectly reordered later, which will be a very subtle bug to detect.  My primary concern with this approach (since I guess we could try to adopt a new MMO policy?) is there's no way for the verifier to catch dropping it.


Repository:
  rL LLVM

https://reviews.llvm.org/D52785