This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] fix BZ 18116 - build failure on ppc64le: setcontext.S uses power6 mtfsf when not supported
- From: Martin Sebor <msebor at redhat dot com>
- To: munroesj at linux dot vnet dot ibm dot com
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Wed, 18 Mar 2015 10:52:24 -0600
- Subject: Re: [PATCH] fix BZ 18116 - build failure on ppc64le: setcontext.S uses power6 mtfsf when not supported
- Authentication-results: sourceware.org; auth=none
- References: <550715C8 dot 7020508 at redhat dot com> <1426538231 dot 13272 dot 28 dot camel at sjmunroe-ThinkPad-W500> <55074888 dot 5060408 at redhat dot com> <55076687 dot 7010903 at redhat dot com> <1426553899 dot 7010 dot 11 dot camel at oc8268013063 dot ibm dot com> <55079FC0 dot 3000806 at redhat dot com> <1426607756 dot 6715 dot 20 dot camel at sjmunroe-ThinkPad-W500>
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