[llvm-commits] [llvm] r140296 - /llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

[Benjamin Kramer]

On 21.09.2011, at 20:32, Bruno Cardoso Lopes wrote:

> Hi Benjamim,
> 
> On Wed, Sep 21, 2011 at 8:27 PM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Wed Sep 21 22:27:22 2011
>> New Revision: 140296
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=140296&view=rev
>> Log:
>> The SSE version differences for fmin/fmax are more involved than I thought.
>> 
>> - x87: no min or max.
>> - SSE1: min/max for single precision scalars and vectors.
>> - SSE2: min/max for single and double precision scalars and vectors.
>> - AVX: as SSE2, but also supports the wider ymm vectors. (this is covered by the isTypeLegal check)
>> 
>> Modified:
>>    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> 
>> Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=140296&r1=140295&r2=140296&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Wed Sep 21 22:27:22 2011
>> @@ -12568,9 +12568,10 @@
>>   // instructions match the semantics of the common C idiom x<y?x:y but not
>>   // x<=y?x:y, because of how they handle negative zero (which can be
>>   // ignored in unsafe-math mode).
>> -  if (Subtarget->hasXMM() &&
>> -      Cond.getOpcode() == ISD::SETCC && VT.isFloatingPoint() &&
>> -      VT != MVT::f80 && DAG.getTargetLoweringInfo().isTypeLegal(VT)) {
>> +  if (Cond.getOpcode() == ISD::SETCC && VT.isFloatingPoint() &&
>> +      VT != MVT::f80 && DAG.getTargetLoweringInfo().isTypeLegal(VT) &&
>> +      (Subtarget->hasXMMInt() ||
>> +       (Subtarget->hasSSE1() && VT.getScalarType() == MVT::f32))) {
>>     ISD::CondCode CC = cast<CondCodeSDNode>(Cond.getOperand(2))->get();
>> 
>>     unsigned Opcode = 0;
> 
> The isTypeLegal already covers not selecting min/maxpd when sse2 isnt available

But it will still select minsd/maxsd and crash :(

The getScalarType() call is indeed superflous, but the checking is required.

- Ben