As my goal is to remove at least _some_ functions from the static list
in MemoryBuiltins.cpp, these tests either need to run inferattrs or
statically declare these attributes to keep passing. A couple of tests
had alternate cases which are no longer meaningful, e.g.
malloc-load-removal.ll.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
60,050 ms | x64 debian > libFuzzer.libFuzzer::large.test |
Event Timeline
I think the general preference would be to explicitly add necessary attributes to test cases, rather than running inferattrs. In some of these cases it's not clear to me why the test would need extra attributes, e.g. in case of llvm/test/Transforms/MemCpyOpt/memcpy.ll -- it does test @malloc, but I don't think it needs anything beyond the noalias attribute that's already present.
Done. A couple of variants of some tests became nonsense (as mentioned in the log message) but for the most part this was straightforward. PTAL?
(This also now comes later in the series, since it has to happen after allockind() exists.)
@nikic where do I stand on this series? Am I waiting on you or should I try and rustle up another reviewer?
This looks good, just wondering whether we can reduce to only allocsize for some of these?
llvm/test/Instrumentation/BoundsChecking/simple.ll | ||
---|---|---|
13 | Do we need more than allocsize for this test? | |
llvm/test/Transforms/InstCombine/objsize-64.ll | ||
5 | Does this and the following test need more than allocsize? | |
llvm/test/Transforms/LowerConstantIntrinsics/builtin-object-size-phi.ll | ||
8 | Does this need more than just allocsize(0)? | |
llvm/test/Transforms/MemCpyOpt/memcpy.ll | ||
331 | Still don't understand why this one would need extra attributes. |
llvm/test/Transforms/InstCombine/objsize-64.ll | ||
---|---|---|
5 | They seem to! If I golf them the tests eventually end up failing later in the series. | |
llvm/test/Transforms/MemCpyOpt/memcpy.ll | ||
331 | I think so that the analysis knows it's an uninitialized block. In any case, if I remove the allockind() the test fails once we're relying on attributes instead of the hard-coded list. |
LGTM
llvm/test/Transforms/MemCpyOpt/memcpy.ll | ||
---|---|---|
331 | Oh duh, we need allockind to allow eliminating the dead malloc call. The allocsize is not necessary. |
Do we need more than allocsize for this test?