Jeff Law <jeffreya...@gmail.com> writes:
On 7/15/2021 4:08 AM, Jiufu Guo via Gcc-patches wrote:
Refine code for V2 according to review comments:
* Use if check instead assert, and refine assert
* Use better RE check for test case, e.g. (?n)/(?p)
* Use better wording for target.def
Currently, doloop.xx variable is using the type as niter which
may be
shorter than word size. For some targets, it would be better
to use
word size type. For example, on 64bit system, to access 32bit
value,
subreg maybe used. Then using 64bit type maybe better for
niter if
it can be present in both 32bit and 64bit.
This patch add target hook for querg perferred mode for doloop
IV.
And update mode accordingly.
Bootstrap and regtest pass on powerpc64le, is this ok for
trunk?
BR.
Jiufu
gcc/ChangeLog:
2021-07-15 Jiufu Guo <guoji...@linux.ibm.com>
PR target/61837
* config/rs6000/rs6000.c (TARGET_PREFERRED_DOLOOP_MODE):
New hook.
(rs6000_preferred_doloop_mode): New hook.
* doc/tm.texi: Regenerate.
* doc/tm.texi.in: Add hook preferred_doloop_mode.
* target.def (preferred_doloop_mode): New hook.
* targhooks.c (default_preferred_doloop_mode): New hook.
* targhooks.h (default_preferred_doloop_mode): New hook.
* tree-ssa-loop-ivopts.c (compute_doloop_base_on_mode):
New function.
(add_iv_candidate_for_doloop): Call
targetm.preferred_doloop_mode
and compute_doloop_base_on_mode.
gcc/testsuite/ChangeLog:
2021-07-15 Jiufu Guo <guoji...@linux.ibm.com>
PR target/61837
* gcc.target/powerpc/pr61837.c: New test.
Hi Jeff,
My first reaction was that whatever type corresponds to the
target's word_mode would be the right choice. But then I
remembered things like dbCC on m68k which had a more limited
range. While I don't think m68k uses the doloop bits, it's a
clear example that the most desirable type may not correspond to
the word type for the target.
I had a check about the implementation of doloop_end insn again.
m68k seems do not implement this insn. On most of these targets
who implement doloop_end insn, word_mode meets the requirement of
doloop_end insn. s390/tilegx and pru are little different. s390
checks SI/DI and arch; tilegx is using if-then-else-jump; on pru,
it seems weird: UNITS_PER_WORD is defined as 1(then word_mode
8bits), while doloop_end rejects QImode.
So, word_mode would be suitable for most targets, while it may not
be best choice for some targets as you said. Then this patch
introducing a hook for target to tune.
So my concern with this patch is its introducing more target
dependencies into the gimple pipeline which is generally
considered undesirable from a design standpoint. Is there any
way to lower from whatever type is chosen by ivopts to the
target's desired type at the gimple->rtl border rather than
doing it in ivopts?
Currently, once gimple choice the type of doloop iv, we keep it
until rtl doloop pass and then some suboptimize (like subreg
access) was living into asm code.
To change the type during lower doloop iv (at expend pass?), we
may also need a hook.
Choosing suitable type early at gimple for doloop iv may avoid
some sub-optimization(e.g. type conversion) for all following
passes.
So, this patch selects the preferred type for doloop iv when it
was generated.
Thanks for your review and further comments!
BR,
Jiufu
jeff