[PATCH] ARM: add VLA extension for WoA Itanium ABI
Saleem Abdulrasool
abdulras at fb.com
Mon Jun 9 10:18:20 PDT 2014
================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:7137
@@ +7136,3 @@
+ case CodeModel::Kernel:
+ BuildMI(*MBB, MI, DL, TII.get(ARM::tBL))
+ .addImm((unsigned)ARMCC::AL).addReg(0)
----------------
Tim Northover wrote:
> Tim Northover wrote:
> > I don't think this is tracking R4 properly. This call is effectively an instruction that should have "R4<imp-use,kill>, R4<imp-def>". Otherwise different phases may assume R4 is unchanged in the interval.
> >
> > Take a look at any normal call to an "i32 @foo(i32)" for a similar example with R0.
> I think an assert or two in this function might be a good idea. At the very least, "isThumb" (instructions that look fine but have the wrong encoding are *really* nasty to track down). Possibly isWindows as well.
I like both ideas! Ive had to track down some cases code that looked right but had the wrong encoding - it is nasty to track down :-(.
================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:7137-7140
@@ +7136,6 @@
+ case CodeModel::Kernel:
+ BuildMI(*MBB, MI, DL, TII.get(ARM::tBL))
+ .addImm((unsigned)ARMCC::AL).addReg(0)
+ .addExternalSymbol("__chkstk")
+ .addReg(ARM::R12, RegState::Implicit | RegState::Define | RegState::Dead);
+ break;
----------------
Saleem Abdulrasool wrote:
> Tim Northover wrote:
> > Tim Northover wrote:
> > > I don't think this is tracking R4 properly. This call is effectively an instruction that should have "R4<imp-use,kill>, R4<imp-def>". Otherwise different phases may assume R4 is unchanged in the interval.
> > >
> > > Take a look at any normal call to an "i32 @foo(i32)" for a similar example with R0.
> > I think an assert or two in this function might be a good idea. At the very least, "isThumb" (instructions that look fine but have the wrong encoding are *really* nasty to track down). Possibly isWindows as well.
> I like both ideas! Ive had to track down some cases code that looked right but had the wrong encoding - it is nasty to track down :-(.
For some reason, I thought I had this tracking R4 usage; guess not. Good catch!
================
Comment at: lib/Target/ARM/ARMInstrInfo.td:5106
@@ +5105,3 @@
+ [SDNPHasChain, SDNPSideEffect]>;
+let usesCustomInserter = 1, Uses = [R4] in
+ def __CHKSTK : PseudoInst<(outs), (ins), NoItinerary, [(__chkstk)]>;
----------------
Tim Northover wrote:
> Tim Northover wrote:
> > Similarly, this should probably be marked "Defs = [R4]".
> Oh no, "Defs = [SP]" probably. Sorry.
Yeah, making this: Uses = [R4], Defs = [R4, SP] seems more precise.
http://reviews.llvm.org/D4050
More information about the llvm-commits
mailing list