On Tue, Apr 28, 2020 at 12:30 AM Sean Christopherson
<sean.j.christopherson@xxxxxxxxx> wrote:
>
> On Mon, Apr 27, 2020 at 10:08:01PM +0200, Uros Bizjak wrote:
> > On Mon, Apr 27, 2020 at 9:25 PM Sean Christopherson
> > <sean.j.christopherson@xxxxxxxxx> wrote:
> > >
> > > On Sun, Apr 26, 2020 at 01:52:55PM +0200, Uros Bizjak wrote:
> > > > Improve handle_external_interrupt_irqoff inline assembly in several ways:
> > > > - use "n" operand constraint instead of "i" and remove
> > >
> > > What's the motivation for using 'n'? The 'i' variant is much more common,
> > > i.e. less likely to trip up readers.
> > >
> > > $ git grep -E "\"i\"\s*\(" | wc -l
> > > 768
> > > $ git grep -E "\"n\"\s*\(" | wc -l
> > > 11
>
> ...
>
> > PUSH is able to use "i" here, since the operand is word wide. But, do
> > we really want to allow symbol references and labels here?
>
> No, but on the other hand, I doubt this particular code is going to change
> much. I don't have a strong preference.
>
> > > > unneeded %c operand modifiers and "$" prefixes
> > > > - use %rsp instead of _ASM_SP, since we are in CONFIG_X86_64 part
> > > > - use $-16 immediate to align %rsp
> > >
> > > Heh, this one depends on the reader, I find 0xfffffffffffffff0 to be much
> > > more intuitive, though admittedly also far easier to screw up.
> >
> > I was beaten by this in the past ... but don't want to bikeshed here.
>
> I'm good with either approach. Same as above, the argument for keeping the
> existing code is that it's there, it works, and from some people it's more
> readable.
Thanks, I'll leave these two discussed points as they were and prepare a v3.
Uros.
