This is an archive of the discontinued LLVM Phabricator instance.

[ValueTracking] Remove volatile check in isGuaranteedToTransferExecutionToSuccessor
ClosedPublic

Authored by uenoku on Jul 28 2019, 5:06 AM.

Details

Summary

As clarified in D53184, volatile load and store do not trap. Therefore, we should remove volatile checks for instructions in isGuaranteedToTransferExecutionToSuccessor.

Diff Detail

Repository
rL LLVM

Event Timeline

uenoku created this revision.Jul 28 2019, 5:06 AM
nikic accepted this revision.Jul 28 2019, 7:35 AM

LGTM per langref:

The compiler may assume execution will continue after a volatile operation, so operations which modify memory or may have undefined behavior can be hoisted past a volatile operation.

llvm/lib/Analysis/ValueTracking.cpp
4268 ↗(On Diff #212091)

Side note: These special-cased intrinsics should be synced now that willreturn exists. In particular assume can be removed from the list because it is already willreturn. sideeffect and experimental.widenable.condition should be marked as willreturn (they must be, otherwise this code would be incorrect) and then removed from here.

llvm/test/Transforms/FunctionAttrs/nonnull.ll
350 ↗(On Diff #212091)

nit: wouldn't -> can't

This revision is now accepted and ready to land.Jul 28 2019, 7:35 AM
uenoku updated this revision to Diff 212159.Jul 29 2019, 6:05 AM

Address comment.

Herald added a project: Restricted Project. · View Herald TranscriptJul 29 2019, 6:05 AM
uenoku marked 3 inline comments as done.Jul 29 2019, 6:06 AM
uenoku added inline comments.
llvm/lib/Analysis/ValueTracking.cpp
4268 ↗(On Diff #212091)

I agree. I'll fix it in another patch.

This revision was automatically updated to reflect the committed changes.
uenoku marked an inline comment as done.