This is an archive of the discontinued LLVM Phabricator instance.

[SDAG] Don't transfer !range metadata without !noundef to SDAG (PR64589)
ClosedPublic

Authored by nikic on Aug 11 2023, 1:18 AM.

Details

Summary

D141386 changed the semantics of !range metadata to return poison on violation. If !range is combined with !noundef, violation is immediate UB instead, matching the old semantics.

In theory, these IR semantics should also carry over into SDAG. In practice, DAGCombine has at least one key transform that is invalid in the presence of poison, namely the conversion of logical and/or to bitwise and/or (https://github.com/llvm/llvm-project/blob/c7b537bf0923df05254f9fa4722b298eb8f4790d/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L11252). Ideally, we would fix this transform, but this will require substantial work to avoid codegen regressions.

In the meantime, avoid transferring !range metadata without !noundef, effectively restoring the old !range metadata semantics on the SDAG layer.

Fixes https://github.com/llvm/llvm-project/issues/64589.

Diff Detail

Event Timeline

nikic created this revision.Aug 11 2023, 1:18 AM
nikic requested review of this revision.Aug 11 2023, 1:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 11 2023, 1:18 AM
DianQK added a subscriber: DianQK.Aug 11 2023, 1:20 AM
arsenm added inline comments.Aug 11 2023, 7:08 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4168

Alternatively could you just insert a freeze if it's not noundef?

nikic updated this revision to Diff 549385.Aug 11 2023, 7:20 AM

Fix a few more codegen tests, looks like I didn't run all last time...

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4168

It's not possible to use range metadata past a freeze, so that would give you the same end result (plus additional regressions from having the freeze).

arsenm accepted this revision.Aug 11 2023, 8:49 AM

GlobalISel probably gets away with this by just not having combines

This revision is now accepted and ready to land.Aug 11 2023, 8:49 AM