Hi Jian-Xin,
Below are some questions/comments:
Why did you change the tsize from 0 to -1? If this is indeed required, then
this is an IR change, and the WHIRL symtab spec should be updated to reflect
this. Also if this needs to change, then a global gatekeeper should approve
this.
In the following change from wgen_spin_symbol.cxx:
@@ -633,6 +629,7 @@
Set_TY_anonymous(ty);
Set_TY_etype (ty, Get_TY (gs_tree_type(type_tree)));
Set_TY_align (idx, TY_align(TY_etype(ty)));
+ TYPE_TY_IDX(type_tree) = idx;
// assumes 1 dimension
// nested arrays are treated as arrays of arrays
ARB_HANDLE arb = New_ARB ();
why did you set TYPE_TY_IDX as above? Because this is anyway set at the end of
this function. Are you setting it because someone queries the IDX of this type
before it's set at the end? Note that this idx at this point may not be the
final IDX, and may be updated with other flags.
Below are some minor comments:
In WGEN_Compose_Address(), you could avoid creating multiple branches each
calling WN_CreateLda. Instead you could set up the offset based on "ofst" and
simplify the code, like you have done in WGEN_Create_Iload() and elsewhere.
Similarly WGEN_Create_Iload() and WGEN_Create_IStore() can be further
simplified to have a single call to WN_CreateIload/WN_CreateIStore by first
setting up the address and offset.
Thanks,
Gautam
________________________________
From: Jian-Xin Lai <laij...@gmail.com>
To: open64-devel <open64-devel@lists.sourceforge.net>
Sent: Tuesday, January 17, 2012 3:18 AM
Subject: [Open64-devel] code review request for bug 933 [WGEN]
Hi,
Could a gatekeeper review the patch for bug #933? Thank you very much.
This patch is used to support GCC extension - variable length array in
struct (VLA/VLS). There are 3 major kinds of changes in the patch:
1. Set the static TY size of VLA/VLS from 0 to -1.
2. Change all component offset in wgen_expr.cxx from WN_OFFSET to WN*.
For constant offset, the operator of WN* is INTCONST. For variable
offset, it's an integer expression.
2. Wrap WN_CreateIload/WN_CreateIIStore/WN_CreateLdid/WN_CreateStid
for variable offset. If the offset is constant, the offset will be
attached onto the WN directly. Otherwise, an ADD node will be
generated for the address used by the load/store.
So far there are still several limitation on this patch:
1. don't support the copy between variable length object.
2. don't support passing the variable length object by parameter or
return value. This one requires to patch the ABI part in BE and is
planed to fix later.
I tested this patch with two methods:
1. Tested the gcc regression. There is no regression found. The
following VLA/VLS test cases can pass now:
gcc.c-torture/compile/20050801-2.c
gcc.c-torture/compile/920428-4.c
gcc.c-torture/compile/920501-16.c
gcc.c-torture/execute/20040308-1.c
gcc.c-torture/execute/20040423-1.c
gcc.c-torture/execute/20041218-2.c
gcc.dg/pr19345.c
gcc.dg/pr30189.c
gcc.dg/vla-2.c
The following cases still fails due to the limitation #2 mentioned above:
gcc.c-torture/compile/20010605-1.c
gcc.c-torture/compile/20030224-1.c
gcc.c-torture/execute/20020412-1.c
gcc.dg/compat/struct-by-value-22
2. Compile the test case without VLA/VLS by different WGEN (one is
original and the other is with this patch) and compare the .B file
dump. There is no difference found in the IR generated by the two
WGENs.
--
Regards,
Lai Jian-Xin
------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
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-d2d
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
------------------------------------------------------------------------------
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
