PATCH: SelectionDAG: Remove setOperationAction(ISD::SELECT_CC, MVT::Other, Expand) hack.
Daniel.Sanders at imgtec.com
Mon May 5 03:03:38 PDT 2014
The two outputs are functionally equivalent. Based on the assembly, the new output appears to be doing:
%0 = select i1 %cond, float 0.0, float 1.0 // lwc1 to load 1.0, mtc1 to copy the hardwired zero to the FPU, and movt.s (move single precision if condition true) to do the select
%0 = select i1 %cond, i32 0, i32 1 // Two addiu's to load 0 and 1, movt to do the select
%1 = sitofp i32 %0 to float // mtc1 to transfer the int to the FPU, and cvt.s.w to convert it to float
I assume the difference stems from the custom lowerings for float/double being ignored before your patches (because of the expansion of MVT::Other) but I haven't tested that.
It's worth mentioning that the 'addiu $1, $zero, 0' in the old code ought to be redundant. It's there because the movt ties the first operand and the result together and writing to $0 discards the result. It could have swapped the first two operands and used movf instead which would have allowed it to use $0 directly.
The old output has slightly better performance on average since it can't suffer a cache miss and would be slightly better again if the redundant instruction were fixed. I doubt it has a significant effect on real code though. If it does turn out to be significant, we can always add special expansions to optimize this case in the float/double lowerings. I therefore think it's ok to update the test case to the new code.
> -----Original Message-----
> From: Tom Stellard [mailto:tom at stellard.net]
> Sent: 02 May 2014 22:36
> To: llvm-commits at cs.uiuc.edu
> Cc: Daniel Sanders; resistor at mac.com
> Subject: PATCH: SelectionDAG: Remove
> setOperationAction(ISD::SELECT_CC, MVT::Other, Expand) hack.
> I've always had a lot of trouble with ISD::SELECT_CC nodes in the
> SelectionDAG, and I think part of the problem is that there is a special case
> for ISD::SELECT_CC where targets can use the MVT::Other type to specify
> one action for all types. The attached patches remove this special case and
> will hopefully help to simplify the legalization and optimization of
> There is one regression on MIPS with these patches:
> I can't tell if this is a real regression or just a change in the order of the code.
> Daniel, could you take a look at this. I have attached the before and after
> output to this email.
More information about the llvm-commits