[PATCH] starting from ARMv8, rGPR includes SP

[Artyom Skrobov]

John, at first I thought the same; but if I just add SP to rGPR,
test/CodeGen/Thumb2/2010-04-15-DynAllocBug.ll shows that alloca gets lowered
into "bic sp, r0, #7", which is unpredictable on ARMv7.
This is because the reservedness of SP doesn't affect the constructions
which are already tied to the stack pointer.

On the other hand, making SP an invalid operand for t2BICri causes the
alloca to get lowered into "bic r0, r0, #7; mov sp, r0" instead.


-----Original Message-----
From: John Brawn 
Sent: 23 June 2015 12:13
To: Artyom Skrobov; llvm-commits at cs.uiuc.edu
Subject: RE: [PATCH] starting from ARMv8, rGPR includes SP

> Is it not possible to do this by actually adding SP to rGPR in the
> RegisterClass definition? There
> would have to be some logic somewhere that then allows it or not based on
> whether the
> subtarget has ARMv8, but I don't know where that would be so maybe that
> approach wouldn't
> work.

I've looked at this a little and I think the closest equivalent to this is
the handling of hasD16,
which is handled by:
 * ARMBaseRegisterInfo::getReservedRegs marks D17-D31 as reserved
 * ARMAsmParser::tryParseRegister rejects D17-D31
 * DecodeDPRRegisterClass in ARMDisassembler.cpp rejects D17-D31

I think if you just add SP to rGPR then actually everything should just
work: SP is already
marked as reserved, and it looks like you already have the necessary changes
to parse and
decode.

John

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of John Brawn
> Sent: 22 June 2015 18:24
> To: Artyom Skrobov; llvm-commits at cs.uiuc.edu
> Subject: RE: [PATCH] starting from ARMv8, rGPR includes SP
> 
> This isn't "rGPR includes SP" it's "rGPR is #defined to GPRnopc", which
> makes things really
> confusing if you're just looking at the .td files to figure out what's
what
> - you see instructions
> that use rGPR, and you see a definition of the rGPR register class, but
> those instructions may
> actually be redefined to use GPRnopc due to something happening elsewhere
> that you
> haven't noticed.
> 
> Is it not possible to do this by actually adding SP to rGPR in the
> RegisterClass definition? There
> would have to be some logic somewhere that then allows it or not based on
> whether the
> subtarget has ARMv8, but I don't know where that would be so maybe that
> approach wouldn't
> work.
> 
> John
> 
> 
> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Artyom Skrobov
> > Sent: 22 June 2015 10:08
> > To: llvm-commits at cs.uiuc.edu
> > Subject: [PATCH] starting from ARMv8, rGPR includes SP
> >
> > Hello,
> >
> > A set of six patches, more or less independent one from another, are
> > attached for review.
> >
> > The people who were last to contribute to the code that my patches are
> > changing are on CC:
> >
> > The first patch fixes a long-standing error in the assembler which
allowed
> > SP and PC as shifted-reg operands in ARMv7 Thumb -- despite correctly
> > rejecting the same registers when not shifted.
> > A test case which appeared to enshrine this incorrect treatment,
> > test/MC/ARM/thumb-shift-encoding.s, was committed by Tim back in 2012;
> > my
> > patch fixes his test, and moves the invalid instructions into
> > test/MC/ARM/thumb2-diagnostics.s
> >
> > The second patch adds a case for MCK_rGPR into
> > ARMAsmParser::validateTargetOperandClass, to allow SP in rGPR, when
> > assembling for ARMv8 Thumb.
> > The third patch, similarly, extends DecoderGPRRegisterClass to allow SP
in
> > rGPR, when disassembling ARMv8 Thumb; and changes
> > DecodeSORegImmOperand to
> > disallow PC (and optionally SP) even when shifted.
> > The existing implementation of DecodeSORegImmOperand was committed
> > by Owen
> > in 2011; and of DecoderGPRRegisterClass, by Amaury in 2013.
> >
> > The next patch includes ARMGenInstrInfo.inc for a second time, after
> > re-defining rGPRRegClassID, in order to enable the codegen to use the
new
> > encodings that become available in ARMv8 Thumb.
> > This patch includes a small tweak to InstrInfoEmitter, so that
> > ARMInstrNameData and ARMInstrNameIndices don't get re-defined.
> > It also requires to make the getXxxDeprecationInfo predicate functions
> > non-static.
> > The code changed by this patch was committed by Tim in 2013.
> >
> > The last two patches are to revert the special case for BXJ,
conditionally
> > accepting either a GPRnopc or a rGRP operand; and to make
> > test/MC/ARM/diagnostics.s and test/MC/ARM/thumb2-diagnostics.s more
> > encompassing, by checking both ARMv7 and ARMv8 handling.
> > The BXJ patch was committed by Charlie, after review by Renato, just two
> > month ago.
> >
> > Any comments on these patches?
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits