[PATCH] D57071: GlobalISel: Handle more cases for widenScalar for G_STORE

[Amara Emerson via Phabricator via llvm-commits]

aemerson accepted this revision.
aemerson added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:946
+    unsigned ExtType = Ty.getScalarSizeInBits() == 1 ?
+      TargetOpcode::G_ZEXT : TargetOpcode::G_ANYEXT;
+    widenScalarSrc(MI, WideTy, 0, ExtType);
----------------
arsenm wrote:
> aemerson wrote:
> > arsenm wrote:
> > > aemerson wrote:
> > > > SelectionDAG does zext for the same IR for all types, so I think we should be consistent there.
> > > I don't think that's true? 
> > > 
> > > In this test case for example, in PromoteIntOp_STORE/PromoteIntRes_LOAD it decided to use the morally equivalent ISD::EXTLOAD extload type.
> > > 
> > > define void @foo(i8 addrspace(1)* %in, i8 addrspace(1)* %out) {
> > >   %val = load i8, i8 addrspace(1)* %in
> > >   store i8 %val, i8 addrspace(1)* %out
> > >   ret void
> > > }
> > > 
> > > The high bits also won't matter for a truncating store
> > So I was using a test case like this to check:
> > 
> > ```
> > define void @test(i12 %v, i12 *%ptr) {
> >   store i12 %v, i12* %ptr
> >   ret void
> > }
> > ```
> > 
> > I get your point that in theory it should be semantically correct to do anyext. However I'd rather not we change the way i1s are handled.
> This is preserving how i1 is handled. It's using zext for i1, and anyext for anything else. I don't think any other non-byte sizes are special cased though
Yes, can you remove the comment about i1s before the code then.


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

https://reviews.llvm.org/D57071