[PATCH] D22635: GlobalISel: add an actual legalizer

Tim Northover via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 22 13:11:39 PDT 2016


t.p.northover accepted this revision.
t.p.northover added a reviewer: t.p.northover.
t.p.northover marked 8 inline comments as done.
t.p.northover added a comment.
This revision is now accepted and ready to land.

Thanks Quentin! Committed with the suggested changes as r276461.


================
Comment at: include/llvm/CodeGen/GlobalISel/MachineIRBuilder.h:158
@@ +157,3 @@
+  /// If \p Ty has size N bits, G_EXTRACT sets \p Res[0] to bits `[Idxs[0],
+  /// Idxs[0] + N)` of \p Src and similarly for subsequent bit-indexes.
+  ///
----------------
qcolombet wrote:
> In my mind the size was not necessarily the same for all extracted operands.
> That being said, we don’t have multiple types for now, nor actual use cases, so this is fine.
Working on it (just whacking a SmallVector into the MachineInstr)! But I'll upgrade G_EXTRACT afterwards.

================
Comment at: lib/CodeGen/GlobalISel/MachineLegalizePass.cpp:49
@@ +48,3 @@
+  // probably want a worklist of instructions rather than naive iterate until
+  // convergence for performance reasons.
+  bool Changed = false;
----------------
qcolombet wrote:
> This may impact the APIs, so maybe we should settle that now.
Soon, but adding enough to test a multi-step legalization would bloat this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D22635





More information about the llvm-commits mailing list