Re: [PATCH] fix BZ 18116 - build failure on ppc64le: setcontext.S uses power6 mtfsf when not supported

[Martin Sebor <msebor at redhat dot com>]

> > You mean around each of the instances, as in the attached patch,
> > as opposed to around the whole #if/#else block?  Would you mind
> > explaining in what way you think it's more correct than the first
> > patch (a single pair of .machine directives around the whole
> > conditional block)?
> I mean around each instance of mtfsf  0xff,fp0,1,0
>
> What you suggested is only correct and safe for IBM POWER hardware
> Power6 and later. It is not safe or correct for powerpc64 chips from
> other manufacturer that implement a subset of the PowerISA-2.05 (or
> later).

I suspect you've misread the changes in the first patch.

The first and the third patches are equivalent and after
preprocessing will result in the assembler seeing the
exact same code, including directives, regardless of what
the target cpu is.

The only difference between the patches is that in the first
one, the .machine "power6" directive is before the #if while
in the third one it's right after it. I.e., in the physical
.S source file, the first patch has half the number of
.machine directives than in the third. That's safe because
they are outside of the prepcoessing conditionals, while in
the third patch they are in each of the #if and #else
branches of the conditionals.

Here is the difference in the first hunk of the setcontext.S
diff between the first patch (directives removed) and the
third (directives added):

  -  .machine push
  -  .machine "power6"

   # ifdef _ARCH_PWR6

  +  .machine push
  +  .machine "power6"

     mtfsf  0xff,fp0,1,0

  +   .machine pop

   # else

     andi.  r6,r5,PPC_FEATURE_HAS_DFP
     beq    5f

  -  .machine push
  -  .machine "power6"

     mtfsf  0xff,fp0,1,0

  -  .machine pop

     b      6f

   5:
     mtfsf  0xff,fp0
   6:

   # endif /* _ARCH_PWR6 */

  +  .machine pop

> Also the *context code is not platform (/powerpc64/swapcontext.S
> vs /powerpc64/power6/swapcontext.S) qualified.
>
> So this code has to work when compiled for older (then power6) processor
> that but is actually running on power6 or later processor that
> implements DPF (PPC_FEATURE_HAS_DFP).

And they both (actually all three of them) will so long
as the assembler has support for the four operand form of
the power6 instruction.

As I said, I don't have a strong preference for which
approach is chosen.  But since you're claiming that the
first patch is incorrect while the third is not I feel
compelled to point out that you're mistaken because they
are, in fact, equivalent.

Martin