Thanks for the explanation. Here is a modified version of the test
program that compiles without error using gcc and shows that 'P' is
accepted as a modifier in the format string.
void test(void)
{
char per_cpu__kernel_stack[100];
void *ret__;
asm volatile ("movq %%gs:%P1,%0"
: "=q" (ret__)
: "p" (&per_cpu__kernel_stack));
}
My best guess for the meaning of the 'P' modifier in this context is
'pointer-sized'. The gcc i386 machine description may shed some light
on it.
The only suggestion I have for your change is to separate the part for
supporting the 'p' constraint from handling the 'P' modifier and make
two check-ins, since they are really two separate problems.
-David Coakey / AMD Open Source Compiler Engineering
On Tue, Nov 15, 2011 at 4:09 PM, Gang Yu <[email protected]> wrote:
> Hi, David
>
> As for the "%P" format string, we directly get it from linux kernel source
> code, a cpped snippet like below:
>
> # 6 "/fc/home/yug/kernel/opencc-build/arch/x86/include/asm/current.h" 2
>
> struct task_struct;
>
> extern __attribute__((section(".discard"), unused)) char
> __pcpu_scope_current_task; extern __attribute__((section(".data.percpu"
> ""))) __typeo\
> f__(struct task_struct *) per_cpu__current_task;
>
> static inline __attribute__((always_inline)) struct task_struct
> *get_current(void)
> {
> return ({ typeof(per_cpu__current_task) ret__; switch
> (sizeof(per_cpu__current_task)) { case 1: asm("mov" "b ""%%""gs"":%P"
> "1"",%0" : "=q" (\
> ret__) : "p" (&per_cpu__current_task)); break; case 2: asm("mov" "w
> ""%%""gs"":%P" "1"",%0" : "=r" (ret__) : "p" (&per_cpu__current_task)); br\
> eak; case 4: asm("mov" "l ""%%""gs"":%P" "1"",%0" : "=r" (ret__) : "p"
> (&per_cpu__current_task)); break; case 8: asm("mov" "q ""%%""gs"":%P" "\
> 1"",%0" : "=r" (ret__) : "p" (&per_cpu__current_task)); break; default:
> __bad_percpu_size(); } ret__; });
> }
>
> it is macro expansion result and not ready to read, so I provide the cpped
> result.
>
> As for the gcc failed bug-cases,
>
> I think you use gcc O0 to test, for the fail reason, because gcc does not
> localize the local char * per_cpu__kernel_stack to a memory location, so
> &per_cpu__kernel_stack is reloaded as a register, that cause the assembler
> assert. open64 does different for this case, so it does well.
>
> If you have further interest, please check out:
>
> https://svn.open64.net/svnroot/open64/regression_test
>
> runtest open64.exp=bug597,bug597-1,bug597-2,....,bug597-9
>
> Thanks
>
> Regards
> Gang
>
>
> On Wed, Nov 16, 2011 at 7:38 AM, David Coakley <[email protected]> wrote:
>>
>> Hi Gang,
>>
>> Your example is slightly more readable if you remove the extra quotes
>> in the format string:
>>
>> void test(void)
>> {
>> char *per_cpu__kernel_stack;
>> void *ret__;
>> asm volatile ("movq %%gs:%P1,%0"
>> : "=q" (ret__)
>> : "p" (&per_cpu__kernel_stack));
>> }
>>
>> You can see "%P" is part of the name "%P1", and 'P' is not documented
>> as a valid modifier. It doesn't compile for me with gcc-4.1.2 or
>> gcc-4.5.3:
>>
>> > gcc -c t.c
>> t.c: Assembler messages:
>> t.c:5: Error: bad memory operand `%rax'
>>
>> It does compile if I remove the 'P'. I understand the part of the bug
>> related to adding support for the 'p' constraint. Are you sure the
>> code related to capital 'P' in the format string is valid for x86?
>>
>> -David Coakley / AMD Open Source Compiler Engineering
>>
>>
>> On Mon, Nov 14, 2011 at 5:40 PM, Gang Yu <[email protected]> wrote:
>> > Thank you, Sun
>> >
>> > As for the inline asm format string such as "%P", I google it but seems
>> > very
>> > few document on it. If people can provide relevant docs or links, it
>> > will be
>> > greatly appreciated, we can further improve it.
>> >
>> > Regards
>> > Gang
>> >
>> >
>> > On Tue, Nov 15, 2011 at 12:18 AM, Sun Chan <[email protected]> wrote:
>> >>
>> >> I am fine with this (although I suspect the use of strstr on %P may be
>> >> not
>> >> covering some error situations)
>> >> Sun
>> >>
>> >> On Mon, Nov 14, 2011 at 8:35 PM, Gang Yu <[email protected]> wrote:
>> >>>
>> >>> Hi,
>> >>>
>> >>> Could a gatekeeper please help review the fix for bug597,
>> >>> constraint
>> >>> <p> not implemented assertion? Thanks very much.
>> >>>
>> >>> Current status:
>> >>>
>> >>> open64 does not implement the "p" asm constraint for x86_64 target,
>> >>> assertion throws out:
>> >>>
>> >>> void test(void)
>> >>> {
>> >>> char *per_cpu__kernel_stack;
>> >>> void *ret__;
>> >>> asm volatile ("mov" "q ""%%""gs"":%P" "1"",%0"
>> >>> : "=q" (ret__)
>> >>> : "p" (&per_cpu__kernel_stack));
>> >>> }
>> >>>
>> >>> ### Assertion failure at line 3323 of
>> >>>
>> >>>
>> >>> /export/proj/linux_test/o64guru/base/trunk/objdir/osprey/../../osprey/be/cg/x8664/cgtarget.cxx:
>> >>> ### Compiler Error in file asm-4.c during Code_Expansion phase:
>> >>> ### ASM constraint <p> not supported
>> >>>
>> >>> The patch below
>> >>>
>> >>> --- a/osprey/be/cg/cgemit.cxx
>> >>> +++ b/osprey/be/cg/cgemit.cxx
>> >>> @@ -3526,13 +3526,17 @@ Modify_Asm_String (char* asm_string, UINT32
>> >>> position, bool memory,
>> >>> asm_string = Replace_Substring(asm_string, replace, name);
>> >>> *name = tmp;
>> >>> }
>> >>> - if (strstr(asm_string, "%P")) {
>> >>> - char replace[5];
>> >>> - sprintf(replace, "%%P%d", position);
>> >>> - // OSP_323, with "%P", we ignore the first character '$'
>> >>> - asm_string = Replace_Substring(asm_string, replace, name+1);
>> >>> - }
>> >>> }
>> >>> + if (strstr(asm_string, "%P")) {
>> >>> + char replace[5];
>> >>> + sprintf(replace, "%%P%d", position);
>> >>> + // OSP_323, with "%P", we ignore the first character '$'
>> >>> + if (*name == '$')
>> >>> + asm_string = Replace_Substring(asm_string, replace, name+1);
>> >>> + else
>> >>> + asm_string = Replace_Substring(asm_string, replace, name);
>> >>> + }
>> >>> +
>> >>> // Follow the zero dialect_number implementation as in
>> >>> // gcc/final.c:output_asm_insn and handle {, } and | operators
>> >>> if (strchr(asm_string, '{')) {
>> >>> diff --git a/osprey/be/cg/x8664/cgtarget.cxx
>> >>> b/osprey/be/cg/x8664/cgtarget.cxx
>> >>> index e006d31..b1c0e58 100644
>> >>> --- a/osprey/be/cg/x8664/cgtarget.cxx
>> >>> +++ b/osprey/be/cg/x8664/cgtarget.cxx
>> >>> @@ -106,6 +106,7 @@
>> >>> #include "cg_loop.h"
>> >>> #include "config_lno.h" // for LNO_Prefetch_Ahead
>> >>> #include "erbe.h"
>> >>> +#include "stblock.h" //for Base_Symbol_And_Offset_For_Addressing
>> >>> @@ -3649,6 +3650,50 @@ CGTARG_TN_For_Asm_Operand (const char*
>> >>> constraint,
>> >>> else
>> >>> ret_tn = (pref_tn ? pref_tn :
>> >>> Build_RCLASS_TN(ISA_REGISTER_CLASS_float));
>> >>> }
>> >>> + else if (*constraint == 'p')
>> >>> + {
>> >>> + FmtAssert(load, (" there must be load expression for constraint
>> >>> p\n"));
>> >>> + if (load && WN_operator(load)==OPR_LDID &&
>> >>> WN_class(load)==CLASS_PREG)
>> >>> + {
>> >>> + // immediate could have been put in preg by wopt
>> >>> + load = Preg_Is_Rematerializable(WN_load_offset(load), NULL);
>> >>> + }
>> >>> + if (WN_operator(load) == OPR_INTCONST)
>> >>> + {
>> >>> + if (Is_Target_32bit() && (WN_const_val(load) > INT32_MAX ||
>> >>> WN_const_val(load) < INT32_MIN)) {
>> >>> + char c[200];
>> >>> + sprintf(c,"%lld", WN_const_val(load));
>> >>> + ErrMsgSrcpos(EC_Ill_Int_Oflow, WN_Get_Linenum(asm_wn),
>> >>> + INT32_MIN,c,INT32_MAX);
>> >>> + }
>> >>> + ret_tn = Gen_Literal_TN(WN_const_val(load),
>> >>> + MTYPE_bit_size(WN_rtype(load))/8);
>> >>> + }
>> >>> + else if ( WN_operator(load) == OPR_LDA &&
>> >>> ST_sym_class(WN_st(load))
>> >>> == CLASS_CONST)
>> >>> + {
>> >>> + ST * base;
>> >>> + INT64 ofst;
>> >>> + // Allocate the string to the rodata section
>> >>> + Allocate_Object (WN_st(load));
>> >>> + Base_Symbol_And_Offset (WN_st(load), &base, &ofst);
>> >>> + ret_tn = Gen_Symbol_TN(base, ofst, 0);
>> >>> + }
>> >>> + else if ( WN_operator(load) == OPR_LDA &&
>> >>> ST_sym_class(WN_st(load))
>> >>> == CLASS_VAR)
>> >>> + {
>> >>> + ST * base;
>> >>> + INT64 ofst;
>> >>> + Base_Symbol_And_Offset_For_Addressing (WN_st(load),
>> >>> WN_lda_offset(load), &base, &ofst);
>> >>> + if (ofst == 0) { //no offset, using a symbol TN
>> >>> + ret_tn = Gen_Symbol_TN(base, ofst, 0);
>> >>> + } else { //has a offset, using a new TN to express the address
>> >>> + ret_tn = (pref_tn ? pref_tn :
>> >>> Build_TN_Of_Mtype(WN_rtype(load)));
>> >>> + }
>> >>> + }
>> >>> + else //other cases
>> >>> + {
>> >>> + ret_tn = (pref_tn ? pref_tn :
>> >>> Build_TN_Of_Mtype(WN_rtype(load)));
>> >>> + }
>> >>> + }
>> >>> else
>> >>> {
>> >>> FmtAssert(FALSE, ("ASM constraint <%s> not supported",
>> >>> constraint));
>> >>>
>> >>> Tests and validation :
>> >>>
>> >>> This patch has passed regression test bug597,bug597-1,...,bug597-9 and
>> >>> there are no such assertions thrown out on the whole kernel(2.6.32-9)
>> >>> build.
>> >>>
>> >>>
>> >>> Regards
>> >>> Gang
>> >>>
>> >>>
>> >>>
>> >>> ------------------------------------------------------------------------------
>> >>> RSA(R) Conference 2012
>> >>> Save $700 by Nov 18
>> >>> Register now
>> >>> http://p.sf.net/sfu/rsa-sfdev2dev1
>> >>> _______________________________________________
>> >>> Open64-devel mailing list
>> >>> [email protected]
>> >>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>> >>>
>> >>
>> >
>> >
>> >
>> > ------------------------------------------------------------------------------
>> > RSA(R) Conference 2012
>> > Save $700 by Nov 18
>> > Register now
>> > http://p.sf.net/sfu/rsa-sfdev2dev1
>> > _______________________________________________
>> > Open64-devel mailing list
>> > [email protected]
>> > https://lists.sourceforge.net/lists/listinfo/open64-devel
>> >
>> >
>
>
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure
contains a definitive record of customers, application performance,
security threats, fraudulent activity, and more. Splunk takes this
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Open64-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/open64-devel
