[PATCH] D40231: InstructionSimplify: 'extractelement' with an undef index is undef

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D40231#936224, @arsenm wrote:

> In https://reviews.llvm.org/D40231#931175, @zvi wrote:
>
> > One option is to run llc with -O0 which will disable passes which use InstructionSimplify, but this will require changing the CHECK's to expect different generated code. If the purpose of the test is to not crash in the Verifier, are the CHECK's needed?
>
>
> The checks probably aren't needed, but if this is eliminated in the IR the test isn't reaching the point it needs to. Is there another way to sneak an undef index in? i.e. some sequence that will fold to undef in the DAG that InstructionSimplify won't catch


Even if we find a loophole in InstSimplify, I think that would be a fragile solution (InstSimplify could get smarter and break the tests again). This could bite us again in the near future because over in https://reviews.llvm.org/D40390, I've suggested adding another simplification for 'insertelement' with an out-of-range index, and indirect-addressing-si.ll has tests that appear to rely on that not folding as well?

Running with -O0 seems like a good option here. Note that there's already a indirect-addressing-si-noopt.ll test file with a FIXME comment:

  ; FIXME: Merge into indirect-addressing-si.ll

...but maybe we should rearrange the tests so that things that are relying on undef are in that file?


https://reviews.llvm.org/D40231