Gang,

We thought it already was a requirement that the SI be provided for
all instructions defined by the ISA.  That is why the x86864 proc
definitions were written that way.  The change in the patch is that
the SI generator now issues an error instead of outputting bad data.

To be more specific, prior to the patch the SI_top_si map could have
NULL entries when there was no SI info provided for a defined
instruction.  Here is an excerpt from sl2_pcore.c generated with trunk
r3462 (just before the patch):

SI * const SI_top_si[774] = {
  &gname103   /* abs16 */,
  &gname103   /* add16 */,
  ...
  &gname103   /* addu */,
  0           /* smult */,
  &gname127   /* div */,
  ...

This is a problem because the access functions defined in ti_si.h do
not expect NULL entries:

inline INT
TSI_Result_Available_Time( TOP top, INT result_index )
{
  return SI_top_si[(INT) top]->result_available_times[result_index];
}

etc.

I am open to relaxing the generator to work more like it did before --
it would output an invalid index instead of a null pointer.  However,
that does not seem consistent with the design.  Perhaps one of the
original authors can provide some guidance.

-David Coakley / AMD Open Source Compiler Engineering

On Mon, Jan 17, 2011 at 6:29 PM, Gang Yu <yugang...@gmail.com> wrote:
> David:
>
>    I have made your patch work under SL. however, I found a potential
> problem on the merge for static psi patch. The new patch does require the
> insn scheduling info full for the defined ISA. however, for simplicity,
> every target is defining an unified ISA for it. It means all processors
> under a target are required defining SI for all insns under the ISA no
> matter the insns are supported or not.
>
>    For example, under the x8664 target, sse4.1 insns are part of the x8664
> ISA. So, in targ_info/proc/x8664/opteron_si.cxx, it must define fake
> scheduling info for sse4.1 which do not exist in the opteron processors. The
> same problem exists in emt64_si.cxx for sse4a not exist in intel arch.
>
>    So, when ISA is extended by the new processors, all legacy scheduling
> infos have to be updated adding fake SI info for sync. SL meets the problem
> now. Could AMD make an update for the patch using isa sub-classification?
> So, we can study the template code and improve it.
>
>   Thanks
>
> Gang
>
>
> On Fri, Jan 14, 2011 at 2:28 PM, David Coakley <dcoak...@gmail.com> wrote:
>>
>> I am still looking for a gatekeeper to review my changes to statically
>> link the target scheduling info.
>>
>> I would like to thank Gang Yu for testing the patch and reporting some
>> problems exposed by the SL target.
>>
>> An updated patch is attached.  Compared to the original patch, the changes
>> are:
>>
>> In targ_info/generate/si_gen.cxx:
>>
>> - Rewrite the incorrect "res_id_set_cmp" comparison function.
>> - Improve the error handling in TOP_SCHED_INFO_MAP::Output_Data.
>>
>> In targ_info/Makefile.gbase:
>>
>> - Fix errors in the PROC_SI_OBJS lists.
>> - Add missing dependencies for correct make -j builds.
>>
>> Also, I corrected some typos in the targ_info/proc/SL/*.cxx files.
>>
>> -David Coakley / AMD Open Source Compiler Engineering
>>
>> On Wed, Jan 5, 2011 at 1:46 PM, David Coakley <dcoak...@gmail.com> wrote:
>> > Hi all,
>> >
>> > I have completed the implementation for my proposal to remove the
>> > scheduling info DSO files and include that data directly into the
>> > backend.  Here is the thread with the original proposal for reference:
>> >
>> >
>> > http://sourceforge.net/mailarchive/forum.php?thread_name=AANLkTik3EfLKqAyXbLnhrBAKMY1VfCiWYd9ssTnzx2oY%40mail.gmail.com&forum_name=open64-devel
>> >
>> > I'm now looking for someone to review/approve the patch.  If there's
>> > anyone familiar with the targ_info subsystem and si_gen.cxx, that's
>> > where the majority of the changes are.
>> >
>> > A few notes:
>> >
>> > 1) Along with some basic smoke testing, I tested that the same code is
>> > generated for x86-64 when building some non-trivial programs.  I do
>> > not expect any changes in the output of the compiler as a result of
>> > this patch.
>> > 2) I also checked the on-disk footprint and compilation time, and saw
>> > no net change in either one.
>> > 3) In osprey/common/targ_info/Makefile.gbase, I removed rules related
>> > to files that do not exist in the source tree, e.g. pentium.so.  If
>> > someone can contribute a pentium_si.cxx, then I'm happy to add the
>> > appropriate rules.  Also, the use of the same filenames in the MIPS/SL
>> > proc subdirectories looks sloppy -- do we really need all the sl*
>> > files in the MIPS directory or r10000_si.cxx in the SL directory?
>> >
>> > -David Coakley / AMD Open Source Compiler Engineering
>> >
>>
>>
>> ------------------------------------------------------------------------------
>> Protect Your Site and Customers from Malware Attacks
>> Learn about various malware tactics and how to avoid them. Understand
>> malware threats, the impact they can have on your business, and how you
>> can protect your company and customers by using code signing.
>> http://p.sf.net/sfu/oracle-sfdevnl
>> _______________________________________________
>> Open64-devel mailing list
>> Open64-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>
>
>

------------------------------------------------------------------------------
Protect Your Site and Customers from Malware Attacks
Learn about various malware tactics and how to avoid them. Understand 
malware threats, the impact they can have on your business, and how you 
can protect your company and customers by using code signing.
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to