[PATCH] D34240: [WebAssembly] Expansion of llvm.umul.overflow with i64 type operands.

[Jatin Bhateja via Phabricator via llvm-commits]

jbhateja added a comment.

In https://reviews.llvm.org/D34240#782070, @efriedma wrote:

> Please make sure to put llvm-commits in the subscriber list when you post a patch.
>
> The changes to call lowering are kind of nasty; I'll sleep on it to see if I come up with a better idea.


Hi Eli, This case is under condition when CanLowerReturn is false, i.e. type is illegal in TargetLowring::LowerCallTo. Also in SelectionDAGLegalize::ExpandNode RTLLIB call is  considered for UMULO operation only when return type is illegal. 
Change has been run through CodeGen regressions which is all clear.



================
Comment at: include/llvm/Target/TargetLowering.h:2802
 
+				bool IsPostTypeLegalization;
+
----------------
vadimcn wrote:
> Indentation.
Will be addressed.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3564
+      BottomHalf = Ret.getOperand(0);
+      TopHalf = Ret.getOperand(1);
     }
----------------
vadimcn wrote:
> So this assumes that the type of node node returned by ExpandLibCall is known and not going to change?   Seems a bit dangerous to me...
Please consider the fact that node being returned from LowerToCall is of type MERGE_VALUES which is a collection of nodes, EXTRACT_ELEMENT is used to extract part value constructed using BUILD_PAIR. BUILD_PAIR makes a larger value type. In this case since call lowering is happening during node legalization (called post type legalization) so returning a collection of stack locations which cumulatively stores the larger result looks fine. We just need to access the operands of MERGE_VALUES to get the Bottom and Top halves.


================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:3564
+      BottomHalf = Ret.getOperand(0);
+      TopHalf = Ret.getOperand(1);
     }
----------------
efriedma wrote:
> jbhateja wrote:
> > vadimcn wrote:
> > > So this assumes that the type of node node returned by ExpandLibCall is known and not going to change?   Seems a bit dangerous to me...
> > Please consider the fact that node being returned from LowerToCall is of type MERGE_VALUES which is a collection of nodes, EXTRACT_ELEMENT is used to extract part value constructed using BUILD_PAIR. BUILD_PAIR makes a larger value type. In this case since call lowering is happening during node legalization (called post type legalization) so returning a collection of stack locations which cumulatively stores the larger result looks fine. We just need to access the operands of MERGE_VALUES to get the Bottom and Top halves.
> Yes, definitely wrong: consider the case where the double-width type is legal.
Hi Eli, This case is under condition when CanLowerReturn is false, i.e. type is illegal in TargetLowring::LowerCallTo. Also in SelectionDAGLegalize::ExpandNode RTLLIB call is  considered for UMULO operation only when return type is illegal. 
Change has been run through CodeGen regressions which is all clear.




https://reviews.llvm.org/D34240