Page MenuHomePhabricator

Provide a more in-depth reference document for TBAA's implementation.
Needs ReviewPublic

Authored by jcranmer-intel on Apr 6 2022, 1:15 PM.

Details

Summary

This new documentation does not describe the "new struct path format" for TBAA.
As far as I am aware, this is only documented in an uncommitted patch
(https://reviews.llvm.org/D40975), and the documentation there is still
incomplete in some key areas.

Diff Detail

Unit TestsFailed

TimeTest
60,080 msx64 debian > libFuzzer.libFuzzer::fuzzer-leak.test
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/LeakTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/fuzzer-leak.test.tmp-LeakTest
60,100 msx64 debian > libFuzzer.libFuzzer::large.test
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/LargeTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/large.test.tmp-LargeTest
60,040 msx64 debian > libFuzzer.libFuzzer::out-of-process-fuzz.test
Script: -- : 'RUN: at line 2'; rm -rf /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/out-of-process-fuzz.test.tmp && mkdir /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/out-of-process-fuzz.test.tmp
60,030 msx64 debian > libFuzzer.libFuzzer::value-profile-load.test
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/LoadTest.cpp -fsanitize-coverage=trace-gep -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/value-profile-load.test.tmp-LoadTest
60,110 msx64 debian > libFuzzer.libFuzzer::value-profile-switch.test
Script: -- : 'RUN: at line 5'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/fuzzer -m64 /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/fuzzer/SwitchTest.cpp -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/value-profile-switch.test.tmp-SwitchTest

Event Timeline

jcranmer-intel created this revision.Apr 6 2022, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 1:15 PM
jcranmer-intel requested review of this revision.Apr 6 2022, 1:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2022, 1:15 PM
xbolva00 added a subscriber: xbolva00.

It looks like this doesn't describe how TBAA differs between load and store operations. It's not as simple as just two memory locations being either "NoAlias" or "MayAlias". Or even if this is correctly describing the implementation, I don't think it's correctly describing how we want it to work. It should be possible to reuse storage for different objects. See, for example, https://github.com/llvm/llvm-project/issues/54878 .

It looks like this doesn't describe how TBAA differs between load and store operations. It's not as simple as just two memory locations being either "NoAlias" or "MayAlias". Or even if this is correctly describing the implementation, I don't think it's correctly describing how we want it to work. It should be possible to reuse storage for different objects. See, for example, https://github.com/llvm/llvm-project/issues/54878 .

FWIW this comes up when running a simple verifier for TBAA results (D123806) over the LLVM code base (for our Small* containers, which re-purpose storage as well).

Looks good. Thanks for documenting this !

I would also (at minimum) mention 'new struct path tbaa' (-new-struct-path-tbaa) which also introduces the size of accesses/fields.

llvm/docs/TBAA.rst
176

s/compnents/components/

Matt added a subscriber: Matt.Jul 20 2022, 12:06 PM