[ARM] Call __chkstk for dynamic stack allocation in all windows environments

[mstorsjo]

Summary

This matches what MSVC does for alloca() function calls on ARM. Even if MSVC doesn't support VLAs at the language level, it does support the alloca function.

On the clang level, both the _alloca() (when emulating MSVC) and __builtin_alloca() builtin functions, and VLAs, map to the same LLVM IR "alloca" function - so within LLVM they're not distinguishable from each other.

[mstorsjo]

Simplified by using Subtarget->isTargetWindows() instead of Subtaget->getTargetTriple().isOSWindows().

[compnerd]

What is the motivation for this change? If you are trying to enable this for MinGW, that is fine, but I'm not sure if we should try to catch the VLA issues in the frontend. I believe that on x86_64, we also disallow the VLAs as Microsoft does not permit them there, but @rnk can correct me if I'm wrong on that.

[mstorsjo]

In D42292#982042, @compnerd wrote:

What is the motivation for this change? If you are trying to enable this for MinGW, that is fine, but I'm not sure if we should try to catch the VLA issues in the frontend. I believe that on x86_64, we also disallow the VLAs as Microsoft does not permit them there, but @rnk can correct me if I'm wrong on that.

I'm trying to enable it both for MinGW and for MSVC. I'm not interested in the case of VLAs though, only for the alloca() function. On the clang level, both are lowered to the same LLVM alloca intrinsic function. to match MSVC better, perhaps clang should reject VLAs on that level if acting in MSVC mode - currently it doesn't. And the current code in LLVM (that I'm changing here) doesn't do much to "disable" VLAs for MSVC - it just lowers them into a normal stack decrement, which would compile just fine, but possibly lead to spurious runtime crashes instead.

[compnerd]

I'm not sure that I follow how this is only for the alloca function. It is for an intrinsic, which just generally enables it. Does it match a cl intrinsic?

[mstorsjo]

In D42292#985281, @compnerd wrote:

I'm not sure that I follow how this is only for the alloca function. It is for an intrinsic, which just generally enables it. Does it match a cl intrinsic?

Yes - this is for an intrinsic, which enables it for both the alloca function and for VLAs - on the LLVM level they're indistinguishable. MSVC supports the alloca function.

If you want to disallow VLAs, that has to be done in clang, not here.

[mstorsjo]

In D42292#985281, @compnerd wrote:

Does it match a cl intrinsic?

In msvc cl.exe, _alloca is an intrinsic that matchea the alloca intrinsic in llvm.

[compnerd]

Ah, then could you add a test case that just lowers the LLVM intrinsic?

[mstorsjo]

Updated the summary/commit message as requested on irc.

llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

ARMTargetLowering::ARMTargetLowering(const TargetMachine &TM,
   // Use the default implementation.
   setOperationAction(ISD::VASTART,            MVT::Other, Custom);
   setOperationAction(ISD::VAARG,              MVT::Other, Expand);
   setOperationAction(ISD::VACOPY,             MVT::Other, Expand);
   setOperationAction(ISD::VAEND,              MVT::Other, Expand);
   setOperationAction(ISD::STACKSAVE,          MVT::Other, Expand);
   setOperationAction(ISD::STACKRESTORE,       MVT::Other, Expand);
 
-  if (Subtarget->getTargetTriple().isWindowsItaniumEnvironment())
+  if (Subtarget->isTargetWindows())
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Custom);
   else
     setOperationAction(ISD::DYNAMIC_STACKALLOC, MVT::i32, Expand);
 
   // ARMv6 Thumb1 (except for CPUs that support dmb / dsb) and earlier use
   // the default expansion.
   InsertFencesForAtomic = false;
   if (Subtarget->hasAnyDataBarrier() &&
SDValue ARMTargetLowering::LowerOperation(SDValue Op, SelectionDAG &DAG) const {
   case ISD::USUBO:
     return LowerUnsignedALUO(Op, DAG);
   case ISD::ATOMIC_LOAD:
   case ISD::ATOMIC_STORE:  return LowerAtomicLoadStore(Op, DAG);
   case ISD::FSINCOS:       return LowerFSINCOS(Op, DAG);
   case ISD::SDIVREM:
   case ISD::UDIVREM:       return LowerDivRem(Op, DAG);
   case ISD::DYNAMIC_STACKALLOC:
-    if (Subtarget->getTargetTriple().isWindowsItaniumEnvironment())
+    if (Subtarget->isTargetWindows())
       return LowerDYNAMIC_STACKALLOC(Op, DAG);
     llvm_unreachable("Don't know how to custom lower this!");
   case ISD::FP_ROUND: return LowerFP_ROUND(Op, DAG);
   case ISD::FP_EXTEND: return LowerFP_EXTEND(Op, DAG);
   case ISD::FPOWI: return LowerFPOWI(Op, *Subtarget, DAG);
   case ARMISD::WIN__DBZCHK: return SDValue();
   }
 }

llvm/trunk/test/CodeGen/ARM/Windows/alloca.ll

 ; RUN: llc -O0 -mtriple thumbv7-windows-itanium -filetype asm -o - %s | FileCheck %s
+; RUN: llc -O0 -mtriple thumbv7-windows-msvc -filetype asm -o - %s | FileCheck %s
+; RUN: llc -O0 -mtriple thumbv7-windows-mingw32 -filetype asm -o - %s | FileCheck %s
 
 declare arm_aapcs_vfpcc i32 @num_entries()
 
 define arm_aapcs_vfpcc void @test___builtin_alloca() {
 entry:
   %array = alloca i8*, align 4
   %call = call arm_aapcs_vfpcc i32 @num_entries()
   %mul = mul i32 4, %call

llvm/trunk/test/CodeGen/ARM/Windows/vla.ll

 ; RUN: llc -mtriple=thumbv7-windows-itanium -mcpu=cortex-a9 -o - %s \
 ; RUN:  | FileCheck %s -check-prefix CHECK-SMALL-CODE
 ; RUN: llc -mtriple=thumbv7-windows-itanium -mcpu=cortex-a9 -code-model=large -o - %s \
 ; RUN:  | FileCheck %s -check-prefix CHECK-LARGE-CODE
 ; RUN: llc -mtriple=thumbv7-windows-msvc -mcpu=cortex-a9 -o - %s \
-; RUN:  | FileCheck %s -check-prefix CHECK-MSVC
+; RUN:  | FileCheck %s -check-prefix CHECK-SMALL-CODE
 
 define arm_aapcs_vfpcc i8 @function(i32 %sz, i32 %idx) {
 entry:
   %vla = alloca i8, i32 %sz, align 1
   %arrayidx = getelementptr inbounds i8, i8* %vla, i32 %idx
   %0 = load volatile i8, i8* %arrayidx, align 1
   ret i8 %0
 }
 
 ; CHECK-SMALL-CODE:   adds [[R4:r[0-9]+]], #7
 ; CHECK-SMALL-CODE:   bic [[R4]], [[R4]], #7
 ; CHECK-SMALL-CODE:   lsrs r4, [[R4]], #2
 ; CHECK-SMALL-CODE:   bl __chkstk
 ; CHECK-SMALL-CODE:   sub.w sp, sp, r4
 
 ; CHECK-LARGE-CODE:   adds  [[R4:r[0-9]+]], #7
 ; CHECK-LARGE-CODE:   bic   [[R4]], [[R4]], #7
 ; CHECK-LARGE-CODE:   lsrs  r4, [[R4]], #2
 ; CHECK-LARGE-CODE:   movw  [[IP:r[0-9]+]], :lower16:__chkstk
 ; CHECK-LARGE-CODE:   movt  [[IP]], :upper16:__chkstk
 ; CHECK-LARGE-CODE:   blx   [[IP]]
 ; CHECK-LARGE-CODE:   sub.w sp, sp, r4
-
-; CHECK-MSVC-NOT: __chkstk
-