Page MenuHomePhabricator

[DAGCombiner] When combining REM ensure optimized div nodes are unique
ClosedPublic

Authored by bsmith on Nov 29 2021, 9:13 AM.

Details

Summary

The REM DAG combine use the visitDivLike functions to try and get an
optimized DIV node to provide better codegen, however in some cases this
visitDivLike call ends up in the BuildSDIVPow2 target hook, which in
turn sometimes will return the same node passed in to indicate not to
change it. The REM DAG combine does not anticipate this and creates a
cycle in the DAG because of it.

Fix this by ensuring any such optimized div node returned is distinct
from the node being combined.

Diff Detail

Event Timeline

bsmith created this revision.Nov 29 2021, 9:13 AM
bsmith requested review of this revision.Nov 29 2021, 9:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 29 2021, 9:13 AM
paulwalker-arm added inline comments.Nov 29 2021, 9:17 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
4439

Can you just use if (OptimizedDiv.getNode() != N) here?

bsmith added inline comments.Nov 29 2021, 9:21 AM
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
4439

Presumably not? If OptimizedDiv is SDValue(), then getNode() will return nullptr which is != N, hence this check would pass, whereas currently it would fail.

paulwalker-arm accepted this revision.Nov 29 2021, 9:30 AM
paulwalker-arm added inline comments.
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
4439

Are yes, I see what you mean. I guess this hasn't been hit before because the code is protected by TLI.isIntDivCheap which is the only other time N is returned untouched.

This revision is now accepted and ready to land.Nov 29 2021, 9:30 AM

Just a post-commit LGTM. I investigated the buildbot failures on the VLA builders, found the same issue and arrived at the exact same solution independently :)