[PATCH] D20443: [PowerPC] - Combine loads of v4i8 to loads of i32 followed by bitcast

Nemanja Ivanovic via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 10:44:37 PDT 2016


nemanjai added inline comments.

================
Comment at: lib/Target/PowerPC/PPCISelLowering.cpp:10569-10582
@@ -10568,2 +10568,16 @@
         return expandVSXLoadForLE(N, DCI);
+
+      // When we load a v4i8, the code can degrade rather quickly. Convert
+      // this to an i32 load and bitcast.
+      if (LoadVT == MVT::v4i8) {
+        SDValue ScalarLoad = DAG.getLoad(MVT::i32, dl, LD->getChain(),
+                                         LD->getBasePtr(), LD->getPointerInfo(),
+                                         false, LD->isNonTemporal(),
+                                         LD->isInvariant(), LD->getAlignment(),
+                                         LD->getAAInfo());
+        SDValue BitCast = DAG.getBitcast(MVT::v4i8, ScalarLoad);
+        return DAG.getNode(ISD::MERGE_VALUES, dl,
+                           DAG.getVTList(MVT::v4i8, MVT::Other),
+                           BitCast, ScalarLoad.getValue(1));
+      }
     }
----------------
amehsan wrote:
> Instead of fixing the issue for this particular pattern, can't we change type legalization, so that it always converts v4i8 to i32? This fixes the problem at hand, but if we have a different code pattern that includes v4i8, the current way of legalizing v4i8 in type legalization will kick in which seems to generate inefficient code, by promoting it to a larger vector and adding permutes and similar instructions.
> 
> I think we may need to change how type legalization handles v4i8, instead of fixing this particular pattern.
Actually, you bring up a very good point. We really should be doing something better with legalization of v4i8 (and I imagine all vectors narrower than vectors our hardware actually handles). However, I don't think we can legalize it as a scalar type. We should actually be widening the vector (rather than promoting the integer element type). I'll re-post this patch to do that instead.


Repository:
  rL LLVM

http://reviews.llvm.org/D20443





More information about the llvm-commits mailing list