On Wed, 2021-03-03 at 21:26 +0100, Ilya Leoshkevich via Gcc-patches
wrote:
> On Wed, 2021-03-03 at 13:02 -0700, Jeff Law wrote:
> > 
> > 
> > On 3/2/21 4:50 PM, Ilya Leoshkevich via Gcc-patches wrote:
> > > Hello,
> > > 
> > > I would like to ping the following patch:
> > > 
> > > Add input_modes parameter to TARGET_MD_ASM_ADJUST hook
> > >  https://gcc.gnu.org/pipermail/gcc-patches/2021-January/562898.html
> > > 
> > > It is needed for the following regression fix:
> > > 
> > > IBM Z: Fix usage of "f" constraint with long doubles
> > >  https://gcc.gnu.org/pipermail/gcc-patches/2021-January/564380.html
> > > 
> > > 
> > > Jakub, who would be the right person to review this change?  I've
> > > decided to ask you, since `git shortlog -ns gcc/cfgexpand.c` shows
> > > that
> > > you deal with this code a lot.
> > > 
> > > Best regards,
> > > Ilya
> > > 
> > > 
> > > 
> > > 
> > > If TARGET_MD_ASM_ADJUST changes a mode of an input operand (which
> > > should be ok as long as the hook itself as well as after_md_seq
> > > make up
> > > for it), input_mode will contain stale information.
> > > 
> > > It might be tempting to fix this by removing input_mode altogether
> > > and
> > > just using GET_MODE (), but this will not work correctly with
> > > constants.
> > > So add input_modes parameter and document that it should be updated
> > > whenever inputs parameter is updated.
> > > 
> > > gcc/ChangeLog:
> > > 
> > > 2021-01-05  Ilya Leoshkevich  <i...@linux.ibm.com>
> > > 
> > >         * cfgexpand.c (expand_asm_loc): Pass new parameter.
> > >         (expand_asm_stmt): Likewise.
> > >         * config/arm/aarch-common-protos.h (arm_md_asm_adjust): Add
> > > new
> > >         parameter.
> > >         * config/arm/aarch-common.c (arm_md_asm_adjust): Likewise.
> > >         * config/arm/arm.c (thumb1_md_asm_adjust): Likewise.
> > >         * config/cris/cris.c (cris_md_asm_adjust): Likewise.
> > >         * config/i386/i386.c (ix86_md_asm_adjust): Likewise.
> > >         * config/mn10300/mn10300.c (mn10300_md_asm_adjust):
> > > Likewise.
> > >         * config/nds32/nds32.c (nds32_md_asm_adjust): Likewise.
> > >         * config/pdp11/pdp11.c (pdp11_md_asm_adjust): Likewise.
> > >         * config/rs6000/rs6000.c (rs6000_md_asm_adjust): Likewise.
> > >         * config/vax/vax.c (vax_md_asm_adjust): Likewise.
> > >         * config/visium/visium.c (visium_md_asm_adjust): Likewise.
> > >         * target.def (md_asm_adjust): Likewise.
> > Ugh.    A couple questions
> > Are there any cases where you're going to want to change modes for
> > arguments that were constants?   I'm a bit surprised that we don't
> > have
> > a mode for constants for the cases that we care about.  Presumably we
> > can get a (modeless) CONST_INT here and we're not restricted to
> > CONST_DOUBLE and friends (which have modes).
> 
> Yes, this might happen.  For example, here:
> 
>     asm("sqxbr\t%0,%1" : "=f"(res) : "f"(0x1.0000000000001p+0L));
> 
> the (const_double) and the corresponding operand will initially have 
> the mode TFmode.  s390_md_asm_adjust () will add a conversion from
> TFmode to FPRX2mode and change the argument accordingly.

Just to be more precise: the mode of the (const_double) itself will not
change.  Here is the resulting RTL for the asm statement above:

# s390_md_asm_adjust () step 1: put the (const_double) operand into a
# new (reg) with the same mode
(insn (set (reg:TF 63)
           (const_double:TF ...)))

# s390_md_asm_adjust () step 2: convert a reg from TFmode to FPRX2mode
(insn (set (reg:FPRX2 65)
           (subreg:FPRX2 (reg:TF 63) 0)))

# s390_md_asm_adjust () step 3: replace the original operand with the
# resulting (reg), adjust (asm_input) accordingly
(insn (set (reg:FPRX2 64)
           (asm_operands:FPRX2 ("sqxbr %0,%1") ("=f") 0
                               [(reg:FPRX2 65)]
                               [(asm_input:FPRX2 ("f"))])))

Reply via email to