[PATCH] D55814: GlobalISel: Support narrowing zextload/sextload

Tue Jan 22 09:42:24 PST 2019

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM

================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:540
+    else
+      MIRBuilder.buildSExt(DstReg, TmpReg);
+
----------------
arsenm wrote:
> dsanders wrote:
> > However, this should be invalid for the same reason, 's32 = G_SEXT s32' doesn't make sense. It would be better for the case where it lowers to G_LOAD to just replace the opcode in place. If we have observers plumbed into the legalizer yet we should also call the changingInstr()+changedInstr() events.
> I don't see why this would actually happen? I suppose that depends on what the constraints are for the specified legalize actions. I don't think you should be allow to specify narrow on a type to the same type. Assuming narrowing a load to the same type isn't allowed, this shouldn't be an issue.
Sorry, I was thinking that if the code above selects the G_LOAD then we can end up with a G_[SZ]EXT that doesn't extend but that can only happen if the G_[SZ]EXTLOAD was invalid in the first place.

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

https://reviews.llvm.org/D55814