This is an archive of the discontinued LLVM Phabricator instance.

[Attributor] Replace AAReturnedValues with AAPotentialValuesReturned
ClosedPublic

Authored by jdoerfert on Jul 10 2023, 10:06 PM.

Details

Summary

The very first AA, at least the first one in order, is not necessary
anymore. AAReturnedValues was from a different time; one might say, a
simpler time.

It was rewriten once to use Attribute::getAssumedSimplifiedValues,
which is what the replacement, AAPotentialValuesReturned, does too.
To match the old behavior we needed to avoid the helper
AAReturnedFromReturnedValues and iterate the return instructions
explicitly, however, it is still less complexity than it was before.
AAReturnedFromReturnedValues and getAssumedSimplifiedValues now
allow users to stop at PHI and select nodes or to ignore those and look
through. AANoFPClass will stop at select and phi nodes to read the
fast math flags.

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

Diff Detail

Event Timeline

jdoerfert created this revision.Jul 10 2023, 10:06 PM
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert requested review of this revision.Jul 10 2023, 10:06 PM
Herald added a project: Restricted Project. · View Herald Transcript

Add nofpclass-select test case (taken from https://github.com/llvm/llvm-project/issues/63404)

Can rebase onto the test addition (plus now all the fences can be dropped)

arsenm added inline comments.Jul 14 2023, 5:15 PM
llvm/test/Transforms/Attributor/nofpclass-select.ll
55–56

Not sure if this should happen here or in a follow up

jdoerfert updated this revision to Diff 540619.Jul 14 2023, 7:45 PM

Remove fences and TODO to show the bug is fixed.

arsenm accepted this revision.Jul 15 2023, 9:05 AM
This revision is now accepted and ready to land.Jul 15 2023, 9:05 AM
This revision was landed with ongoing or failed builds.Jul 17 2023, 10:44 AM
This revision was automatically updated to reflect the committed changes.
hctim added a subscriber: hctim.Jul 17 2023, 2:35 PM

Hi Johannes, looks like this broke the UBSan buildbot: https://lab.llvm.org/buildbot/#/builders/85/builds/17728/steps/13/logs/stdio

Full instructions to repro the bot are available at https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild. You'd be looking to use the buildbot_bootstrap_ubsan.sh script in that folder. But you can probably do something with cmake -DLLVM_USE_SANITIZER=Undefined -DCMAKE_C_FLAGS="-fsanitize=undefined" -DCMAKE_CXX_FLAGS="-fsanitize=undefined" < ... > && ninja check-llvm.

An example breakage posted below for your convenience:

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70
FAIL: LLVM :: Transforms/Attributor/value-simplify-dominance.ll (55751 of 75828)
******************** TEST 'LLVM :: Transforms/Attributor/value-simplify-dominance.ll' FAILED ********************
Script:
--
: 'RUN: at line 2';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/opt -aa-pipeline=basic-aa -passes=attributor -attributor-manifest-internal  -attributor-annotate-decl-cs  -S < /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/test/Transforms/Attributor/value-simplify-dominance.ll | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck --allow-unused-prefixes /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/test/Transforms/Attributor/value-simplify-dominance.ll --check-prefixes=CHECK,TUNIT
: 'RUN: at line 3';   /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/opt -aa-pipeline=basic-aa -passes=attributor-cgscc -attributor-manifest-internal  -attributor-annotate-decl-cs -S < /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/test/Transforms/Attributor/value-simplify-dominance.ll | /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm_build_ubsan/bin/FileCheck --allow-unused-prefixes /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/test/Transforms/Attributor/value-simplify-dominance.ll --check-prefixes=CHECK,CGSCC
--
Exit Code: 2
Command Output (stderr):
--
/b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:1414:32: runtime error: load of value 255, which is not a valid value for type 'bool'
    #0 0x558bf8720d98 in llvm::Attributor::getAssumedSimplifiedValues(llvm::IRPosition const&, llvm::AbstractAttribute const*, llvm::SmallVectorImpl<llvm::AA::ValueAndContext>&, llvm::AA::ValueScope, bool&, bool) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:1414:32
    #1 0x558bf87b9cbc in (anonymous namespace)::AAPotentialValuesReturned::updateImpl(llvm::Attributor&)::'lambda'(llvm::Value&, llvm::Instruction*, bool)::operator()(llvm::Value&, llvm::Instruction*, bool) const /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11367:16
    #2 0x558bf8727b39 in operator() /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/ADT/STLFunctionalExtras.h:68:12
    #3 0x558bf8727b39 in checkForAllInstructionsImpl /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:1961:12
    #4 0x558bf8727b39 in llvm::Attributor::checkForAllInstructions(llvm::function_ref<bool (llvm::Instruction&)>, llvm::Function const*, llvm::AbstractAttribute const&, llvm::ArrayRef<unsigned int> const&, bool&, bool, bool) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:1986:8
    #5 0x558bf8727d95 in llvm::Attributor::checkForAllInstructions(llvm::function_ref<bool (llvm::Instruction&)>, llvm::AbstractAttribute const&, llvm::ArrayRef<unsigned int> const&, bool&, bool, bool) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:2002:10
    #6 0x558bf87b9687 in (anonymous namespace)::AAPotentialValuesReturned::updateImpl(llvm::Attributor&) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/AttributorAttributes.cpp:11394:14
    #7 0x558bf871d62c in llvm::AbstractAttribute::update(llvm::Attributor&) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:1016:16
    #8 0x558bf872be9f in llvm::Attributor::updateAA(llvm::AbstractAttribute&) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/lib/Transforms/IPO/Attributor.cpp:2616:13
    #9 0x558bf8721366 in llvm::AAPotentialValues const* llvm::Attributor::getOrCreateAAFor<llvm::AAPotentialValues>(llvm::IRPosition, llvm::AbstractAttribute const*, llvm::DepClassTy, bool, bool) /b/sanitizer-x86_64-linux-bootstrap-ubsan/build/llvm-project/llvm/include/llvm/Transforms/IPO/Attributor.h:1593:7

I also see failures of under asan caused by this commit