[PATCH] D129357: [LoongArch] Implement OR combination to generate bstrins.w/d

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added a comment.

The logic looks good, thanks.

I have several more real-world test cases (LoongArch and IA64 code generation) but I plan to submit them after this lands.



================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:570
+  // Similarly, the 7th pattern is more common than the 2nd. So we put the 2nd
+  // before the 7th so that to match as many nodes as possible.
+  if (N1.getOpcode() == ISD::AND &&
----------------
nit: "in order to match" for consistency with the wording in the 6th pattern above.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchISelLowering.cpp:603
+  // Similarly, the 8th pattern is more common than the 4th and 5th patterns. So
+  // I put it here so that to match as many nodes as possible or generate less
+  // instructions.
----------------
nit: "we put it here in order to match"


================
Comment at: llvm/test/CodeGen/LoongArch/bstrins_d.ll:175
+;; Pattern 8: a = b | (c & shifted_mask)
+;; Similar to pattern but without shift to c.
+define i64 @pat8(i64 %c) nounwind {
----------------
typo: "pattern 7"


================
Comment at: llvm/test/CodeGen/LoongArch/bstrins_w.ll:183
+;; Pattern 8: a = b | (c & shifted_mask)
+;; Similar to pattern but without shift to c.
+define i32 @pat8(i32 %c) nounwind {
----------------
ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129357