Page MenuHomePhabricator

[Attributor] Don't modify functions outside of the current SCC
AcceptedPublic

Authored by aeubanks on Apr 7 2021, 5:09 PM.

Diff Detail

Unit TestsFailed

TimeTest
250 msx64 debian > LLVM.Transforms/Attributor::align.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=9 -S < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/align.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/align.ll --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
330 msx64 debian > LLVM.Transforms/Attributor::liveness.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=35 -S < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/liveness.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/liveness.ll --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
280 msx64 debian > LLVM.Transforms/Attributor::nonnull.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=14 -S < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/nonnull.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/nonnull.ll --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
160 msx64 debian > LLVM.Transforms/Attributor::potential.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -enable-new-pm=0 -attributor -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=20 -S < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/potential.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/potential.ll --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
340 msx64 debian > LLVM.Transforms/Attributor::range.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/opt -attributor -enable-new-pm=0 -attributor-manifest-internal -attributor-max-iterations-verify -attributor-annotate-decl-cs -attributor-max-iterations=23 -S < /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/range.ll | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck --allow-unused-prefixes /mnt/disks/ssd0/agent/llvm-project/llvm/test/Transforms/Attributor/range.ll --check-prefixes=CHECK,NOT_CGSCC_NPM,NOT_CGSCC_OPM,NOT_TUNIT_NPM,IS__TUNIT____,IS________OPM,IS__TUNIT_OPM
View Full Test Results (32 Failed)

Event Timeline

aeubanks created this revision.Apr 7 2021, 5:09 PM
aeubanks requested review of this revision.Apr 7 2021, 5:09 PM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
jdoerfert accepted this revision.Apr 7 2021, 5:13 PM

Thanks a lot for tracking this down!

LGTM, two nits though: can we include the reproducer as test please, and below.

llvm/include/llvm/Transforms/IPO/Attributor.h
1346

Can you add a doxygen comment here describing when/why we sometimes cannot change a use.

This revision is now accepted and ready to land.Apr 7 2021, 5:13 PM

I'm not actually sure that this is the right change, it was more of a "does something like this make sense"

Updating the tests, this is doing something wrong since it's replacing a bunch of values with undef. We'd probably need to look at all of the callers to see if they're doing the right thing.

I'm not actually sure that this is the right change, it was more of a "does something like this make sense"

Updating the tests, this is doing something wrong since it's replacing a bunch of values with undef. We'd probably need to look at all of the callers to see if they're doing the right thing.

I don't follow, sorry.