Re: [Xen-devel] [PATCH] x86: use alternatives for FS/GS base accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.09.18 at 20:13, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 26/09/18 07:43, Jan Beulich wrote:
>> Eliminating conditional branches is always a Good Thing (tm), it seems to 
> me.
> 
> By this reasoning, we should compile Xen with movfuscator, which really
> will get rid of every branch.
> 
> Doing so would be utter nonsense, ergo this claim is false.

Please take the use of "always" here with a grain of salt: Of course
I'm not suggesting to eliminate all conditionals. But conditionals which
are there sole to select between different insns achieving the same
thing are a pretty good target for replacement by alternatives.

>> And that's not just for
>> performance (inside Xen we can't assume at all that any code path,
>> even the context switch one, is hot enough to have any BTB entries
>> allocated), 
> 
> This is a valid argument, for why the proposed change might plausibly be
> an improvement.
> 
> It is by no means a guarantee that making the change will result in
> improved performance.

As I've said before - if improved code clarity is accompanied by no
drop in performance, that would already be good enough imo. And
I see no reason to believe that streamlining code by eliminating
conditional branches has any meaningful risk of impacting
performance, without the need to do any measurements (and I
say this despite realizing that at times actual behavior can be
surprising).

>> but also for ease of looking at the assembly, should there
>> be a need to do so.
> 
> Using alternatives actively obfuscates the logic in the disassembly.  It
> is almost impossible to distinguish the individual fragments, and you
> rejected my suggestion of rectifying this by putting symbols into the
> .altinstructions section.

I could be talked into this, provided they don't make it into the
"kallsyms"-like symbol table.

>  It also results in harder to read C, and

This may or may not be the case, depending on which part of
the code you look at. Especially if there are conditionals in
mainline code, their elimination helps readability imo. But yes,
there's _some_ price to pay.

> poorer surrounded code generation, as the compiler has to cope with the
> union of entry/exit requirements for the blocks.

Interesting - you now use an argument the inverse of which you
use on the PDEP/PEXT patch series.

Furthermore I question this argument in the context here: In
order to satisfy the WRMSR constraints, the compiler is left with
basically no choice of register allocation already anyway. The
replacement insns then simply use these same registers. It's
certainly possible that without alternatives the non-WRMSR
part of the conditional uses different register allocation, but
according to my (general, not specific to this case here)
observations gcc at least is often producing more redundancy
than would be possible to go with, in order to use different
registers in cases like this.

> This patch still hasn't addressed the concerns about sh[lr]d, and the
> resulting competition for execution resource on AMD Fam15/16h systems.

Excuse me? Where did you see any SH[LR]D left in the patch?

> "x86: enable interrupts earlier with XPTI disabled" was objected to by
> me on the basis of the increased complexity of following the code,
> rather than any performance consideration.  A contributory factor was
> that I couldn't see any reason why it would make any performance
> difference.  When Juergen eventually measured it, the results said the
> performance was worse.  (It might be interesting to work out why it was
> worse overall, because its definitely not obvious, but I suspect we all
> have more important work to do).

And I've accepted your arguments (without fully buying them, with
latency in mind) and dropped the patch.

> "x86: use PDEP/PEXT for maddr/direct-map-offset conversion when
> available​" neglects the cache bloat of having 255 copies of the stub,
> and the pipeline stall from mixing legacy and VEX SSE instructions. 

I've responded to both of these before. I'm not going to repeat any
of that here.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel