[PATCH] D32746: [globalisel][legalizer] G_LOAD/G_STORE NarrowScalar should not emit G_GEP x, 0.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:247
+
+      MIRBuilder.materializeGEP(SrcReg, MI.getOperand(1).getReg(), LLT::scalar(64), Adjustment);
 
----------------
qcolombet wrote:
> Is this clang-formated?
> In the browser it looks larger than 80-col, but it may just be an impression.
I've fixed this in the commit.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:247
+
+      MIRBuilder.materializeGEP(SrcReg, MI.getOperand(1).getReg(), LLT::scalar(64), Adjustment);
 
----------------
igorb wrote:
> dsanders wrote:
> > qcolombet wrote:
> > > Is this clang-formated?
> > > In the browser it looks larger than 80-col, but it may just be an impression.
> > I've fixed this in the commit.
> could you calculate offset type base on pointer size ?  So it will be legal for 32bit targets .
Sure, I've made this change in the commit.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:280
+      } else
+        DstReg = MI.getOperand(1).getReg();
+
----------------
qcolombet wrote:
> Shouldn't we call materializeGEP here too for the whole if-then block?
I've fixed this in the commit.


https://reviews.llvm.org/D32746