[PATCH] D108757: [X86][Codegen] PR51615: don't replace wide volatile load with narrow broadcast-from-memory

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:5047
+  return !Ld->isVolatile() ||
+         Ld->getValueSizeInBits(0) == EltVT.getScalarSizeInBits();
+}
----------------
lebedev.ri wrote:
> spatel wrote:
> > I'm not sure if the semantics on this are settled - what happens if the overall size matches, but the "shape" isn't the same?
> > I'm imagining something like:
> >   %i = load volatile <2 x float>, <2 x float>* @test5_id12345, align 16
> >   %b = bitcast <2 x float> %i to <1 x double>
> >   %i1 = shufflevector <1 x double> %b, <1 x double> poison, <4 x i32> <i32 undef, i32 0, i32 undef, i32 0>
> I don't see why the "shape" would matter. it's either a single load of the same size or it's not.
> https://llvm.org/docs/LangRef.html#volatile-memory-accesses notes
> > <...> the backend should never split or merge target-legal volatile load/store instructions. Similarly, IR-level volatile loads and stores cannot change from integer to floating-point or vice versa.
It's that clause about changing types that raises the question. Ie, if it's not allowable to change from `float` to `i32` for example, then is it also not allowable to change from `<1 x double>` to `<1 x i64>`?

The restriction seems to be limited to IR for some reason, so maybe it is irrelevant here, but seems worth adding another test, so we have some marker for current behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108757