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. However, this is not the problematic case that I refer to in the commit message: I caught some failures in the testsuite that I tracked down to (const_int)s, which, like you mentioned, don't have a mode. > Is input_modes read after the call to md_asm_adjust? I'm trying to > figure out why we'd need to update it. Yes, its contents goes into (asm_operand)'s (asm_input). If we don't adjust it, (asm_input)s will no longer be consistent with input operand RTXes. > Not acking or naking at this point, I just want to make sure I > understand what's going on. > > jeff