[PATCH] D17414: Fix incorrect selection of AVX512 sqrt when OptForSize is on

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 09:31:15 PST 2016


My understanding is that the other cases are ignored: it's a bug but not a correctness one.

-- 
Mehdi

Sent from my iPhone

> On Feb 19, 2016, at 9:09 AM, Roman Divacky <rdivacky at vlakno.cz> wrote:
> 
> What about the other cases of Requires[OptForSize] in the same file?
> 
>> On Thu, Feb 18, 2016 at 10:11:59PM +0000, Dimitry Andric via llvm-commits wrote:
>> dim created this revision.
>> dim added reviewers: joker.eph, grosbach, resistor.
>> dim added a subscriber: llvm-commits.
>> 
>> When optimizing for size, sqrt calls can be incorrectly selected as
>> AVX512 VSQRT instructions.  This is because X86InstrAVX512.td has a
>> `Requires<[OptForSize]>` in its `avx512_sqrt_scalar` multiclass
>> definition.  Even if the target does not support AVX512, the class can
>> apparently still be chosen, leading to an incorrect selection of
>> `vsqrtss`.
>> 
>> In PR26625, this lead to an assertion: Reg >= X86::FP0 && Reg <=
>> X86::FP6 && "Expected FP register!", because the `vsqrtss` instruction
>> requires an XMM register, which is not available on i686 CPUs.
>> 
>> http://reviews.llvm.org/D17414
>> 
>> Files:
>>  lib/Target/X86/X86InstrAVX512.td
>>  test/CodeGen/X86/pr26625.ll
>> 
>> Index: test/CodeGen/X86/pr26625.ll
>> ===================================================================
>> --- /dev/null
>> +++ test/CodeGen/X86/pr26625.ll
>> @@ -0,0 +1,20 @@
>> +; RUN: llc < %s -mcpu=i686 2>&1 | FileCheck %s
>> +; PR26625
>> +
>> +target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
>> +target triple = "i386"
>> +
>> +define float @x0(float %f) #0 {
>> +entry:
>> +  %call = tail call float @sqrtf(float %f) #1
>> +  ret float %call
>> +; CHECK-LABEL: x0:
>> +; CHECK: flds
>> +; CHECK-NEXT: fsqrt
>> +; CHECK-NOT: vsqrtss
>> +}
>> +
>> +declare float @sqrtf(float) #0
>> +
>> +attributes #0 = { nounwind optsize readnone }
>> +attributes #1 = { nounwind optsize readnone }
>> Index: lib/Target/X86/X86InstrAVX512.td
>> ===================================================================
>> --- lib/Target/X86/X86InstrAVX512.td
>> +++ lib/Target/X86/X86InstrAVX512.td
>> @@ -6005,7 +6005,7 @@
>> 
>>   def : Pat<(_.EltVT (OpNode (load addr:$src))),
>>             (!cast<Instruction>(NAME#SUFF#Zm)
>> -                (_.EltVT (IMPLICIT_DEF)), addr:$src)>, Requires<[OptForSize]>;
>> +                (_.EltVT (IMPLICIT_DEF)), addr:$src)>, Requires<[HasAVX512, OptForSize]>;
>> }
>> 
>> multiclass avx512_sqrt_scalar_all<bits<8> opc, string OpcodeStr> {
> 
>> Index: test/CodeGen/X86/pr26625.ll
>> ===================================================================
>> --- /dev/null
>> +++ test/CodeGen/X86/pr26625.ll
>> @@ -0,0 +1,20 @@
>> +; RUN: llc < %s -mcpu=i686 2>&1 | FileCheck %s
>> +; PR26625
>> +
>> +target datalayout = "e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128"
>> +target triple = "i386"
>> +
>> +define float @x0(float %f) #0 {
>> +entry:
>> +  %call = tail call float @sqrtf(float %f) #1
>> +  ret float %call
>> +; CHECK-LABEL: x0:
>> +; CHECK: flds
>> +; CHECK-NEXT: fsqrt
>> +; CHECK-NOT: vsqrtss
>> +}
>> +
>> +declare float @sqrtf(float) #0
>> +
>> +attributes #0 = { nounwind optsize readnone }
>> +attributes #1 = { nounwind optsize readnone }
>> Index: lib/Target/X86/X86InstrAVX512.td
>> ===================================================================
>> --- lib/Target/X86/X86InstrAVX512.td
>> +++ lib/Target/X86/X86InstrAVX512.td
>> @@ -6005,7 +6005,7 @@
>> 
>>   def : Pat<(_.EltVT (OpNode (load addr:$src))),
>>             (!cast<Instruction>(NAME#SUFF#Zm)
>> -                (_.EltVT (IMPLICIT_DEF)), addr:$src)>, Requires<[OptForSize]>;
>> +                (_.EltVT (IMPLICIT_DEF)), addr:$src)>, Requires<[HasAVX512, OptForSize]>;
>> }
>> 
>> multiclass avx512_sqrt_scalar_all<bits<8> opc, string OpcodeStr> {
> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 


More information about the llvm-commits mailing list