[PATCH] D73201: [FPEnv][AArch64] Don't expand STRICT_FP_ROUND to an illegal truncating store

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D73201#1834300 <https://reviews.llvm.org/D73201#1834300>, @john.brawn wrote:

> In D73201#1834254 <https://reviews.llvm.org/D73201#1834254>, @craig.topper wrote:
>
> > So does AArch64 set FP_ROUND to Legal or Custom? STRICT_FP_ROUND should do the same.
>
>
> FP_ROUND is Custom in AArch64. I could add something to the AArch64 target to handle this, but it's a bit odd that the default behaviour is for an invalid operation to be generated (and I see that -mtriple=sparc hits the same assertion failure, as it also has FP_ROUND set to Custom).


STRICT_ contains a lot of hacks to try to guess what to do by trying to see what the target does for non-strict nodes. That's why there's a call to getStrictFPOperationAction which will query the non-strict behavior. But it doesn't handle Custom well because we don't know what to do with it.

The change you've proposed here will end up causing the STRICT_FP_ROUND node to be considered Legal even though its set to Expand. It will then be mutated to the non-strict node during isel which is technically wrong. And in this case it works because there are isel patterns for most of the FP_ROUND combinations since the Custom handler only does something special for fp128. So fp128 STRICT_FP_ROUND will end up failing isel since it really needs to be Custom handled.

We want to remove all the mutation code once the in tree targets all support STRICT_ nodes properly. It's a hack to enable targets to limp along a little bit, but in hindsight may have been a bad idea since it creates the illusion of things working when they really don't.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73201/new/

https://reviews.llvm.org/D73201