Thanks for Jian-xin's review.
Below is the improved patch, with no absolute%%ebx in the asm_string.
index e75ebdc..54aa950 100644
--- a/osprey/be/cg/cgemit.cxx
+++ b/osprey/be/cg/cgemit.cxx
@@ -3350,8 +3350,20 @@ Modify_Asm_String (char* asm_string, UINT32
position, bool memory,
#endif
if( base_ofst == 0 ){
#if defined(TARG_X8664)
- if( Is_Target_32bit() )
- sprintf( buf, "%s", name );
+ // open64.net bug951.
+ // Format IA32 GOT symbol in Asm_String.
+ if( Is_Target_32bit() ) {
+ switch (TN_relocs(tn)) {
+ case TN_RELOC_IA32_GOT:
+ sprintf( buf,
+ "%s@GOT(%s)",
+ name,
+ int_reg_names[3][TN_register(Ebx_TN())-
REGISTER_MIN]);
+ break;
+ default:
+ sprintf( buf, "%s", name);
+ }
+ }
else
sprintf( buf, "%s(%%rip)", name );
#else
@@ -3359,8 +3371,21 @@ Modify_Asm_String (char* asm_string, UINT32
position, bool memory,
#endif
} else
#if defined(TARG_X8664)
- if( Is_Target_32bit() )
- sprintf( buf, "%s+%d", name, (int)base_ofst );
+ // open64.net bug951.
+ // Format IA32 GOTOFF symbol in Asm_String.
+ if( Is_Target_32bit() ) {
+ switch (TN_relocs(tn)) {
+ case TN_RELOC_IA32_GOTOFF:
+ sprintf( buf,
+ "%s@GOTOFF+%d(%s)",
+ name,
+ (int)base_ofst,
+ int_reg_names[3][TN_register(Ebx_TN())-
REGISTER_MIN]);
+ break;
+ default:
+ sprintf( buf, "%s+%d", name, (int)base_ofst );
+ }
+ }
else
sprintf( buf, "%s+%d(%%rip)", name, (int)base_ofst );
#else
diff --git a/osprey/be/cg/x8664/cgtarget.cxx
b/osprey/be/cg/x8664/cgtarget.cxx
index 89f89d6..e78cbf9 100644
--- a/osprey/be/cg/x8664/cgtarget.cxx
+++ b/osprey/be/cg/x8664/cgtarget.cxx
@@ -3908,7 +3908,15 @@ TN* CGTARG_Process_Asm_m_constraint( WN* load,
void** offset, OPS* ops )
if( WN_operator(load) == OPR_LDA ){
OP* lda_op = OPS_last( ops );
- asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) : OP_opnd( lda_op, 0
);
+ // open64.net bug951. On finding the symbol TN, don't miss the cases
of:
+ // TN :- ld32 GTN2(%rbx) (sym:base_sym +0)
+ // TN :- lea32 GTN2(%rbx) (sym:base_sym+base_ofst)
+ if (Is_Target_32bit() && Gen_PIC_Shared) {
+ asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) :
+ (OP_code(lda_op) == TOP_ldc32) ? OP_opnd( lda_op, 0) : OP_opnd(
lda_op, 1);
+ } else {
+ asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) : OP_opnd( lda_op,
0 );
+ }
OPS_Remove_Op( ops, lda_op );
} else if( WN_operator(load) == OPR_ADD ){
diff --git a/osprey/be/cg/x8664/exp_loadstore.cxx
b/osprey/be/cg/x8664/exp_loadstore.cxx
index 99bde2c..58cb61e 100644
--- a/osprey/be/cg/x8664/exp_loadstore.cxx
+++ b/osprey/be/cg/x8664/exp_loadstore.cxx
@@ -1267,17 +1267,29 @@ Exp_Ldst (
STB_section(base_sym) /* bug 10097 */)) ){
FmtAssert(!ST_is_thread_local(base_sym),
("Exp_Ldst: thread-local storage NYI under PIC"));
- TN* tmp = base_ofst == 0 ? tn : Build_TN_Like(tn);
- Build_OP( TOP_ld32, tmp, Ebx_TN(),
- Gen_Symbol_TN( base_sym, 0, TN_RELOC_IA32_GOT ),
- &newops );
+
+ // open64.net bug951.
+ // when base_ofst ! = 0, we generate
+ // TN :- lea32 GTN2(%rbx) (sym:base_sym+base_ofst)
+ // instead of
+ // tmp_TN :- ld32 GT2(%bx) (sym:base_sym + 0)
+ // TN :- lea32 tmp_TN base_ofst
+ // this is for consideration of facilitate restoring the
register TN
+ // to symbol TN in ASM m constraint.
+ // And also this reduces a temp TN.
+
+ if (base_ofst == 0)
+ Build_OP( TOP_ld32, tn, Ebx_TN(),
+ Gen_Symbol_TN( base_sym, base_ofst,
TN_RELOC_IA32_GOT ),
+ &newops );
+ else
+ Build_OP( TOP_lea32, tn, Ebx_TN(),
+ Gen_Symbol_TN( base_sym, base_ofst,
TN_RELOC_IA32_GOTOFF ),
+ &newops );
// got address should not alias
Set_OP_no_alias(OPS_last(&newops));
PU_References_GOT = TRUE;
- if( base_ofst != 0 ){
- Build_OP( TOP_lea32, tn, tmp, Gen_Literal_TN(base_ofst, 4),
&newops );
- }
} else {
Build_OP( TOP_ldc32, tn,
Gen_Symbol_TN( base_sym, base_ofst, TN_RELOC_NONE ),
Regards
Gang
On Fri, Feb 24, 2012 at 2:30 PM, Gang Yu <yugang...@gmail.com> wrote:
> Hi,
>
> Could a gatekeeper please help review the fix for bug951(
> http://bugs.open64.net/show_bug.cgi?id=951)?
>
> The cut-down bugcase:
> typedef union {
> } GenefxAccumulator;
>
> struct _GenefxState {
> int length;
> void *Aop[3];
> int AopY;
> GenefxAccumulator *Dacc;
> GenefxAccumulator *Sacc;
> };
>
> typedef struct _GenefxState GenefxState;
> int main(void)
> {
> GenefxState *gfxs;
> static const u32 preload[] = { 0xFF00FF00, 0x0000FF00 };
> static const u32 mask[] = { 0x00FC00F8, 0x000000F8 };
> static const u32 pm[] = { 0x01000004, 0x00000004 };
> __asm__ __volatile__ (
> "movq %3, %%mm7\n\t"
> :
> : "D" (gfxs->Aop[0]), "c" (gfxs->length), "S" (gfxs->Sacc),
> "m" (*preload), "m" (*mask), "m" (*pm)
> : "%eax", "%st", "memory");
> return 0;
> }
>
> opencc -m32 -fpic -S bug951.c
> and opencc asserts:
> ### Assertion failure at line 3377 of
> /fc/proj/ctires/open64/o64guru/src/Thu/trunk/osprey/be/cg/cgemit.cxx:
> ### Compiler Error in file bug951.c during Assembly phase:
> ### ASM operand must be a register or a numeric constant
>
> Analysis:
> The CGIR after code expansion phase for the asm statement is below:
>
> [ 58, 0] TN150 :- ld32 TN4(%rsp) (sym:gfxs+0) ; WN id:17 gfxs+0x0
> [ 58, 0] TN5(%rdi) :- ld32 TN150 (0x8) ; WN id:23
> [ 58, 0] TN8(%rcx) :- ld32 TN150 (0x0) ; WN id:24
> [ 58, 0] TN6(%rsi) :- ld32 TN150 (0x1c) ; WN id:25
> [ 58, 0] TN153 :- ld32 TN2(%rbx) (sym:.data.rel.ro.local+0) ; noalias WN
> [ 58, 0] TN155 :- ld32 TN2(%rbx) (sym:.data.rel.ro.local+0) ; noalias WN
> [ 58, 0] :- asm TN5(%rdi) TN8(%rcx) TN6(%rsi) TN2(%rbx) (0x10) (0x20) ;
> volatile side_effects
> -----------------------------------------------------------------------
>
> Obviously the asm stmt is unexpected, 0x10 as literal TN should not be "m"
> constraint. "m" constraint in the ASM statement which is a constant address
> should be symbol tn, not register tn, nor literal tn.
>
> The expected result should be
> [ 22, 0] TN149 :- ld32 TN4(%rsp) (sym:gfxs+0) ; WN id:9 gfxs+0x0
> [ 22, 0] TN5(%rdi) :- ld32 TN149 (0x4) ; WN id:10
> [ 22, 0] TN8(%rcx) :- ld32 TN149 (0x0) ; WN id:11
> [ 22, 0] TN6(%rsi) :- ld32 TN149 (0x18) ; WN id:12
> [ 22, 0] :- asm TN5(%rdi) TN8(%rcx) TN6(%rsi) (sym:.data.rel.ro.local+0)
> (sym:.data.rel.ro.local+16) (sym:.data.rel.ro.local+32) ; volatile
> side_effects
>
> Investigate shows we miss the -m32 LDA_handling cases in
> CGTARG_Asm_m_constraint.
> asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) : OP_opnd( lda_op, 0 );
> does not work for all cases.
>
> Suggested fix:
>
> use TN_RELOC_IA32_GOTOFF instead of TN_RELOC_IA32_GOT to load addresses
> for symbols with offset, so to match the handling in ASM_m_constraint.
>
> below is the patch:
>
> index e75ebdc..ba246fd 100644
> --- a/osprey/be/cg/cgemit.cxx
> +++ b/osprey/be/cg/cgemit.cxx
> @@ -3350,8 +3350,17 @@ Modify_Asm_String (char* asm_string, UINT32
> position, bool memory,
> #endif
> if( base_ofst == 0 ){
> #if defined(TARG_X8664)
> - if( Is_Target_32bit() )
> - sprintf( buf, "%s", name );
> + // open64.net bug951.
> + // Format IA32 GOT symbol in Asm_String.
> + if( Is_Target_32bit() ) {
> + switch (TN_relocs(tn)) {
> + case TN_RELOC_IA32_GOT:
> + sprintf( buf, "%s@GOT(%%ebx)", name );
> + break;
> + default:
> + sprintf( buf, "%s", name);
> + }
> + }
> else
> sprintf( buf, "%s(%%rip)", name );
> #else
> @@ -3359,8 +3368,17 @@ Modify_Asm_String (char* asm_string, UINT32
> position, bool memory,
> #endif
> } else
> #if defined(TARG_X8664)
> - if( Is_Target_32bit() )
> - sprintf( buf, "%s+%d", name, (int)base_ofst );
> + // open64.net bug951.
> + // Format IA32 GOTOFF symbol in Asm_String.
> + if( Is_Target_32bit() ) {
> + switch (TN_relocs(tn)) {
> + case TN_RELOC_IA32_GOTOFF:
> + sprintf( buf, "%s@GOTOFF+%d(%%ebx)", name, (int)base_ofst);
> + break;
> + default:
> + sprintf( buf, "%s+%d", name, (int)base_ofst );
> + }
> + }
> else
> sprintf( buf, "%s+%d(%%rip)", name, (int)base_ofst );
> #else
> diff --git a/osprey/be/cg/x8664/cgtarget.cxx
> b/osprey/be/cg/x8664/cgtarget.cxx
> index 89f89d6..e78cbf9 100644
> --- a/osprey/be/cg/x8664/cgtarget.cxx
> +++ b/osprey/be/cg/x8664/cgtarget.cxx
> @@ -3908,7 +3908,15 @@ TN* CGTARG_Process_Asm_m_constraint( WN* load,
> void** offset, OPS* ops )
>
> if( WN_operator(load) == OPR_LDA ){
> OP* lda_op = OPS_last( ops );
> - asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) : OP_opnd( lda_op,
> 0 );
> + // open64.net bug951. On finding the symbol TN, don't miss the cases
> of:
> + // TN :- ld32 GTN2(%rbx) (sym:base_sym +0)
> + // TN :- lea32 GTN2(%rbx) (sym:base_sym+base_ofst)
> + if (Is_Target_32bit() && Gen_PIC_Shared) {
> + asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) :
> + (OP_code(lda_op) == TOP_ldc32) ? OP_opnd( lda_op, 0) : OP_opnd(
> lda_op, 1);
> + } else {
> + asm_opnd = OP_iadd(lda_op) ? OP_opnd( lda_op, 1 ) : OP_opnd(
> lda_op, 0 );
> + }
> OPS_Remove_Op( ops, lda_op );
>
> } else if( WN_operator(load) == OPR_ADD ){
> diff --git a/osprey/be/cg/x8664/exp_loadstore.cxx
> b/osprey/be/cg/x8664/exp_loadstore.cxx
> index 99bde2c..58cb61e 100644
> --- a/osprey/be/cg/x8664/exp_loadstore.cxx
> +++ b/osprey/be/cg/x8664/exp_loadstore.cxx
> @@ -1267,17 +1267,29 @@ Exp_Ldst (
> STB_section(base_sym) /* bug 10097 */)) ){
> FmtAssert(!ST_is_thread_local(base_sym),
> ("Exp_Ldst: thread-local storage NYI under PIC"));
> - TN* tmp = base_ofst == 0 ? tn : Build_TN_Like(tn);
> - Build_OP( TOP_ld32, tmp, Ebx_TN(),
> - Gen_Symbol_TN( base_sym, 0, TN_RELOC_IA32_GOT ),
> - &newops );
> +
> + // open64.net bug951.
> + // when base_ofst ! = 0, we generate
> + // TN :- lea32 GTN2(%rbx) (sym:base_sym+base_ofst)
> + // instead of
> + // tmp_TN :- ld32 GT2(%bx) (sym:base_sym + 0)
> + // TN :- lea32 tmp_TN base_ofst
> + // this is for consideration of facilitate restoring the
> register TN
> + // to symbol TN in ASM m constraint.
> + // And also this reduces a temp TN.
> +
> + if (base_ofst == 0)
> + Build_OP( TOP_ld32, tn, Ebx_TN(),
> + Gen_Symbol_TN( base_sym, base_ofst,
> TN_RELOC_IA32_GOT ),
> + &newops );
> + else
> + Build_OP( TOP_lea32, tn, Ebx_TN(),
> + Gen_Symbol_TN( base_sym, base_ofst,
> TN_RELOC_IA32_GOTOFF ),
> + &newops );
> // got address should not alias
> Set_OP_no_alias(OPS_last(&newops));
> PU_References_GOT = TRUE;
>
> - if( base_ofst != 0 ){
> - Build_OP( TOP_lea32, tn, tmp, Gen_Literal_TN(base_ofst, 4),
> &newops );
> - }
> } else {
> Build_OP( TOP_ldc32, tn,
> Gen_Symbol_TN( base_sym, base_ofst, TN_RELOC_NONE ),
>
> TIA for the review.
>
> Regards
> Gang
>
------------------------------------------------------------------------------
Try before you buy = See our experts in action!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-dev2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
