Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I drafted a similar patch, and I think removing the saved-state complexity is worth it.
The other possibility -- and this patch doesn't have to be gated on this, but it could save a lot of back-and-forth -- is to set up a FastMathFlagGuard object like we have in IRBuilder (adding reviewers from D87037).
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
4875–4877 | Should use the incoming Flags now. | |
4889 | Should use the incoming Flags on all created nodes now. | |
5010 | Should use the incoming Flags on all created nodes now. | |
5085 | Should use the incoming Flags on all created nodes now. | |
5197 | Should use the incoming Flags on all created nodes now. | |
8956 | Could get rid of the FMF temp and use SDFlags everywhere below. | |
llvm/test/CodeGen/PowerPC/fmf-propagation.ll | ||
560 | We need to find where this flag got dropped? |
llvm/test/CodeGen/PowerPC/fmf-propagation.ll | ||
---|---|---|
560 | It was getting a FMF flag before because it was the last instruction created by handling a call. So the after the fact handling found it. I can’t think of a reason we need FMF on a copy. If we need them anywhere it would be on the CALL_NOP right? |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
5010 | Can we do that in a follow up? There's a quite a bit of code in these approximations. Adding the flags could enable FMA formation or reassociation that we weren't doing before. |
LGTM, but please wait at least a day to commit in case there are other comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
5010 | Yes - but on 2nd thought, we probably shouldn't do that on any of these expansions. IIRC, we had to undo the FMF propagation on some of the x86 cast expansions to avoid bungling the expected bit-hacks. We should just assume these expansions are the best as-is. | |
llvm/test/CodeGen/PowerPC/fmf-propagation.ll | ||
560 | Yes, that makes sense. If we need a flag, it should be on the call, so that's independent of this patch. |
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | ||
---|---|---|
5010 | Could still update the TODO comments though - ie, we are now intentionally not propagating FMF because these are hand-crafted expansions, and we don't expect the code to be altered further. |
clang-format not found in user's PATH; not linting file.