[PATCH] D30401: Refactor the multiply-accumulate combines to act on ARMISD::ADD[CE] nodes, instead of the generic ISD::ADD[CE].

[A. Skrobov via Phabricator via llvm-commits]

tyomitch added inline comments.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9451
   //                    ADDC   <- hiAdd
   //
+  assert(AddeNode->getOpcode() == ARMISD::ADDE && "Expect an ADDE");
----------------
samparker wrote:
> I think it would be a good idea to update this comment with the new pattern.
The pattern remains unchanged; it's the namespace of ADDC and ADDE nodes (which isn't reflected in this comment) which changed.


================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:9563
   // While trying to combine for the other MLAL nodes, first search for the
   // chance to use UMAAL. Check if Addc uses another addc node which can first
   // be combined into a UMLAL. The other pattern is AddcNode being combined
----------------
samparker wrote:
> just another small comment update required.
Thanks for pointing my attention to this one! Indeed, with the new arrangement, `ARMTargetLowering::PerformDAGCombine` can do all necessary combines, so that the "second pass" in `ARMDAGToDAGISel::Select` would no longer be necessary.


https://reviews.llvm.org/D30401