[PATCH] D23930: [AArch64] Fix encoding for lsl #12 in add/sub immediates

[Diana Picus via llvm-commits]

rovka added a comment.

In https://reviews.llvm.org/D23930#528671, @rengolin wrote:

> In https://reviews.llvm.org/D23930#528639, @peter.smith wrote:
>
> > From my reading of the code it seems like we'll fault this as out of range. If I'm right it would be nice to support this, but I don't think it necessarily needs to be done as part of this one.
>
>
> Yes, this is an interesting enhancement, and should be reasonably simple. But not for this patch, I agree.


I don't know if it's that simple. The logic that actually computes the value to be stored in the immediate is part of the fixup handling, and at that point we're really only concerned with those 12 bits that represent the immediate (the shift bit is not part of the fixup). I can't think of a good way to handle this without having to extend the fixup to cover the shift bit as well, which is kind of awkward because then we'd have to express the shift as part of Expr (so we'd be creating a new shift expression with the original Expr as an operand, and then parsing that as part of the fixup). I'm not sure it's worth the effort.

Also, I think it would be difficult to maintain assembly that relies on this property: .space 0x1000 works, but .space 0x1000 - 4 gives an "immediate out of range" error, which just looks surprising unless you figure out to inspect the encodings.

Just my 2 cents.


https://reviews.llvm.org/D23930