[PATCH] D147678: [DAGCombiner][AArch64] Use scalar_to_vector to eliminate bitcast

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added a comment.

My gut feeling is this isn't a valid transformation.  Looking at the output from:

  define <2 x i16> @bitcast_from_int(i32 %word) {
    %ret = bitcast i32 %word to <2 x i16>
    ret <2 x i16> %ret
  }
  
  define i16 @extract_lo(<2 x i16> %vec) {
    %ret = extractelement <2 x i16> %vec, i64 0
    ret i16 %ret
  }
  
  define i16 @extract_hi(<2 x i16> %vec) {
    %ret = extractelement <2 x i16> %vec, i64 1
    ret i16 %ret
  }

  bitcast_from_int:                       // @bitcast_from_int
  	sub	sp, sp, #16
  	.cfi_def_cfa_offset 16
  	add	x8, sp, #12
  	str	w0, [sp, #12]
  	ld1	{ v0.h }[0], [x8]
  	orr	x8, x8, #0x2
  	ld1	{ v0.h }[2], [x8]
  	add	sp, sp, #16
  	ret
  
  extract_lo:                             // @extract_lo
  	fmov	w0, s0
  	ret
  
  extract_hi:                             // @extract_hi
  	mov	w0, v0.s[1]
  	ret

Shows `<2 x i16>` is represented as an unpacked `<2 x i32>` vector with the valid bits being the bottom 16-bits within each 32bit element.  This means the `ISD::ANY_EXTEND` within the DAG your matching is critical and this gets dropped when replacing with `ISD::SCALAR_TO_VECTOR`.  Ultimately it looks like `ISD::BITCAST` for smaller than legal vectors types is not a nop unless the src/dst is a load/store in which case the extension can be removed.  The poor code is the result of type legalisation so perhaps you'd be better off implementing custom legalisation for `ISD::BITCAST` involving 32-bit vector types.  I recall we did something similar for SVE to handle its unpacked vector types (see ReplaceBITCASTResults).


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

https://reviews.llvm.org/D147678