This is an archive of the discontinued LLVM Phabricator instance.

[Instcombine] Try to get select's FMFs from its operands
AbandonedPublic

Authored by mnadeem on Oct 21 2021, 5:06 PM.

Details

Summary

This tries to tackle https://bugs.llvm.org/show_bug.cgi?id=52259

Intersect the FMF of the select's operands. If the operand is not a FPMathOperator e.g. an argument then try to take the flag from the function's attributes.

Diff Detail

Event Timeline

mnadeem created this revision.Oct 21 2021, 5:06 PM
mnadeem requested review of this revision.Oct 21 2021, 5:06 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2021, 5:06 PM

Using function attributes is what we're trying to get away from by having instruction-level FMF, so this would have to be clearly labeled as a hack or temporary fix.

That said, I don't think anyone is working on a real fix that would apply FMF to all FP values. That was a comment I made in:
https://llvm.org/PR35607
...but it might lead to another question about how that works with type-less memory (I don't know the answer to that one).

I think that's also the root cause for the failure in the motivating bug/test for this patch:
https://llvm.org/PR52259
That is, I don't think we can blame SROA for not propagating FMF when creating a phi - the FMF weren't on the loaded values used to form that phi in the first place.

This fabs example is worth posting to llvm-dev for wider discussion. I think we do need to do something to optimize that code, but there's no clear fix that I know of.
Note that this example is really only about 'nsz':
https://godbolt.org/z/1zW6hK1qK

lebedev.ri resigned from this revision.Jan 12 2023, 5:24 PM

This review seems to be stuck/dead, consider abandoning if no longer relevant.

Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2023, 5:24 PM
Herald added a subscriber: StephenFan. · View Herald Transcript
mnadeem abandoned this revision.Jan 13 2023, 8:54 PM