[PATCH] D55722: [DAGCombiner] scalarize binop followed by extractelement

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: test/CodeGen/ARM/vector-promotion.ll:16
+; ASM: orr r0, r0, #1
+; ASM-NEXT: str r0, [r1]
 ; ASM-NEXT: bx
----------------
efriedma wrote:
> This is destroying the whole point of the test: we want to avoid the expensive float->int register transfer.
So this raises what appears to be a show-stopper for this patch. We have this reverse transform in IR, so we'd have to redo that as a late DAG transform (we'd have 2 reversals to get back to the code we started with). I don't think that I can cleanly use the TLI hook that was added for the CGP transform because it takes IR type params.

But this ties into something I noticed in InstCombine/DAGCombine. We have a DAG function called scalarizeExtractedVectorLoad() (except there seems to be a bug in how it is called - it's not used with a constant extract index). 

I was wondering why we don't do that as an IR canonicalization. That would appear to make both this test and the next diff moot - we would remove all vector ops right from the start. Do you see a problem with that?

To make that clearer, I think the ideal ARM asm for this test has no vector ops:
	ldr	r0, [r0, #4]
	orr	r0, r0, #1
	str	r0, [r1]
	bx	lr



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55722/new/

https://reviews.llvm.org/D55722