Depends on D108371
|50 ms||x64 debian > LLVM.Bindings/Go::go.test|
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/llvm-go go=/usr/bin/go test llvm.org/llvm/bindings/go/llvm
|170 ms||x64 debian > LLVM.Transforms/LoopVectorize/X86::vectorization-remarks-missed.ll|
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/opt < /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/LoopVectorize/X86/vectorization-remarks-missed.ll -loop-vectorize -transform-warning -S -pass-remarks-missed='loop-vectorize' -pass-remarks-analysis='loop-vectorize' -enable-new-pm=0 2>&1 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/llvm/test/Transforms/LoopVectorize/X86/vectorization-remarks-missed.ll
This change is to address the comment: https://reviews.llvm.org/D108371#inline-1093152
I've updated the debug info in the test to point the loop to
Instead of passing in the pointer with a default value of nullptr, can you make it a reference, so that it always gets set, e.g.
That also removes the need for the conditional write.
Is this an unrelated change?
Can we please leave it as initialised as nullptr ?
The conditional write ensures that the first unsafe dependence gets reported by the remark.
if (UncomputablePtr) *UncomputablePtr = Access.getPointer();
Ok. I've submitted as a separate commit to avoid any confusion.
Ok please ignore the above. I've made the change as suggested,
But I didn't remove that bit in the code.
if (!UncomputablePtr) UncomputablePtr = Access.getPointer();
That should be the choice of the caller, not the callee. If the caller doesn't want the value to be overwritten, it should pass a dummy variable
I believe this check should be removed (see my other comment as well)
This variable aliases with UncomputablePtr defined on line 2075. Either pass in the same variable, or give this variable a different name.
Before this change, recordAnalysis was called unconditionally, but now it is only called if UncomputablePtr is set. I don't think that's intentional, so you can instead just do this:
auto *I = dyn_cast_or_null<Instruction>(UncomputablePtr); recordAnalysis("CantIdentifyArrayBounds", I) << "cannot identify array bounds";
nit: I'm not really a fan of the name, so personally I would prefer to reuse the old variable, e.g. reset it first:
UncomputablePtr = nullptr;
and then pass that into canCheckPtrAtRT.