[llvm-commits] Request for Review - avoid register pressure with read/modify/write instructions

[Evan Cheng]

Comments:

I know some of the issues were there before your patch. But please correct them while you are at it.

Can you add some comments to isLdIncOrDecSt? There should be at least top level comments. Also I think there is a bug in this. Please check if the load is a non-indexed, non-extending load (using isNormalLoad()). Similarly, please  make sure the store is not a truncating store.

int memSize = StoreNode->getMemOperand()->getSize();
Please do not depend on memoperand for this. LSBaseSDNode has MemVT field. Or better yet, just use Load's ValueType. So replace

+    if (!(memSize == 8 || memSize == 4 || memSize == 2 || memSize == 1)) 
+        return false;

with something like:
if (LdVT != MVT::i8 && LdVT != MVT::i16 && LdVT != MVT::i32 && LdVT != MVT::i64)
  return false;

newIncDecOpc() has some formatting issues. Indentation width should be 2.

One more nitpick. The names for these functions look weird. How about isLoadIncDecStore? getFusedLdStOpcode?

Evan

On Mar 26, 2012, at 6:01 PM, Joel Jones <joel_k_jones at apple.com> wrote:

> This is a code change to add support for changing instruction sequences of the form:
>  load
>  inc/dec of 8/16/32/64 bits
>  store
> into the appropriate X86 inc/dec instruction:
>  inc[qlwb]/dec[qlwb]
> 
> The change is in lib/Target/X86/X86ISelDAGToDAG.cpp, an svn diff of which is attached.
> Also attached is a test case.
> 
> Joel Jones
> 
> <X86OISelDAGToDAG.cpp.diff><readModWrite.ll>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits