[LLVMdev] Possible bug in getCallPreservedMask for CallingConv::Intel_OCL_BI

[Duncan P. N. Exon Smith]

On 2014 Mar 14, at 09:20, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:

> +elena.demikhovsky at intel.com, who added the tests in question.
> 
> On 2014 Mar 13, at 10:30, Philip Reames <listmail at philipreames.com> wrote:
> 
>> Not sure who owns this bit of code, so sending this to the general list.
>> 
>> It looks like there may be an unintentional fall through happening in the X86RegisterInfo::getCallPreservedMask function.
>> 
>> http://llvm.org/docs/doxygen/html/X86RegisterInfo_8cpp_source.html
>> 
>> case CallingConv::Intel_OCL_BI
>> : {
>> 
>> if
>> (IsWin64 && HasAVX512)
>> 
>> return
>> CSR_Win64_Intel_OCL_BI_AVX512_RegMask;
>> 
>> if
>> (Is64Bit && HasAVX512)
>> 
>> return
>> CSR_64_Intel_OCL_BI_AVX512_RegMask;
>> 
>> if
>> (IsWin64 && HasAVX)
>> 
>> return
>> CSR_Win64_Intel_OCL_BI_AVX_RegMask;
>> 
>> if
>> (Is64Bit && HasAVX)
>> 
>> return
>> CSR_64_Intel_OCL_BI_AVX_RegMask;
>> 
>> if
>> (!HasAVX && !IsWin64 && Is64Bit)
>> 
>> return
>> CSR_64_Intel_OCL_BI_RegMask;
>>     // Missing break/return?
>> }
>> 
>> It _may_ be intentional - it falls through to the Cold calling convention and then general C convention, but it really looks suspicious.  
>> 
>> If it is intentional, please add an explicit note about the desired fall through behaviour.  I found this by inserting a new case in the switch and having tests fail.  The two tests that seem to hit the fallthrough behavor are: CodeGen/X86/avx-intel-ocl.ll and CodeGen/X86/sse-intel-ocl.ll
>> 
>> Philip
> 
> It looks like a bug to me.  getCalleeSavedRegs() should have essentially identical logic, and there’s a break there.
> 
> I’ve committed r203934 and r203939 which both should have NFC.  I’ll construct a testcase that exposes the problem with the missing break statement; once I have that, I can commit that change too.
> 
> Elena, does this sound right to you?

I went ahead and committed r203943 without a testcase.  The accidental fallthrough is not observable.