From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x629.google.com (mail-pl1-x629.google.com [IPv6:2607:f8b0:4864:20::629]) by sourceware.org (Postfix) with ESMTPS id B3E1E3858429 for ; Tue, 12 Dec 2023 07:14:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B3E1E3858429 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org B3E1E3858429 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::629 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702365281; cv=none; b=GHY3mV0mr4Msed1emZh2sNtgyFJM20wuu49C/npKv2vHZ/8wHBmXuJS8fGNmUbREGGJsEiv+s7osvslx77CEf71KWj7VYAOrsglofet2F/KGsUBCfxHT6qNFlI+7MruQjnjm9DXB8LwKiBAyot53rxDKAO7elqvsRVoHIrzLe7s= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702365281; c=relaxed/simple; bh=Avvz7cpqbY4/L/4OMZHCfl6Ed/hq3Xx/RzLesGne21I=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=EyrQ4r2EpL9KTeyG5sWYuG8kPnbcbBatnel1LFd2ubCxAjYobzkx8NdY/l4FoBQcG+o56jIApjDgmOL7ay2w/OJreFaBAddZC8CcDFyU08s8DQIaNSFi+T0Wv9aAyzLK35S0Sk0loZfS1UetO2/HwBn5PRF5OnMjrOVwONmG3wo= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x629.google.com with SMTP id d9443c01a7336-1d337dc9697so2619065ad.3 for ; Mon, 11 Dec 2023 23:14:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1702365279; x=1702970079; darn=gcc.gnu.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=tEvo3gkxUMf/IRRQkPnEpvZ2ukUvLA3m5z8+VbMxhVg=; b=cDq/UAXYs7xZMhyTKm+axqnkzbSmogmAWL98wGL60H5s7oTjCclq7RrzlOOtShHuRr jIQqYwlx8ak/az18ZGPnhKTuLbtxOWJNaWppGnTiUGVkZsbECzw9TYHKHhxsQyEBj1aa cLETvmNo4KonV8DMzU2KwT4Pv6JjVWdISmbRpmRrsifWywJLVBVj4JMhyXeYLOO81Okz FopRF6sKHEY/V+64hrWnm7WIP4IgJ0dty174GSeqqRiVNbZVTS9a94pQOJI4bevMo4jw HElTjN6VL9yzx3lnC7L94W8aUD9k9m/+pLgoAGtgVQYQ/u28J1pQeDmE7+772vXPHrNd g4tg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702365279; x=1702970079; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=tEvo3gkxUMf/IRRQkPnEpvZ2ukUvLA3m5z8+VbMxhVg=; b=DlqcfyuI2SwWvRcffOR0o0xvgiRdQ9UWJhwJ3VHILFiGykCuIDPhq+z4MHbLOznWxh k5cGewrEmPrfDPG1jD87bNqreu0ur2BzE6xxlrSMKJzs2CdhyKawx4HaqslHeZPl6R49 EXdtfGguF2BCIzXoEUWmuOkIIOG1ztlX7fEpaRlp5dBJ9C7ccO3XGTqKawlHK9ynGcW2 50rBlXJMt9PslokwjHfF/oSLT4SkZ9ASp9d999LWIx+5ltGJjdMeUJQnNSDFXciK9RiS 81wy0OP+wlqfi47p6J7Q05DhGmgZVEJbiwS3k6n2rTx27I8Rt1ryUA4iNPUaiVPZCK3p bh8g== X-Gm-Message-State: AOJu0YzL8H4qjSYwkcMhlCv32r5i8Qh7s6yR5fH6O+0vVFatWEg/FeCE 9bBTP/eDvA9xOnsAINuUhBbtQTRkyHJvoephj0w= X-Google-Smtp-Source: AGHT+IFAu2ACGXwecyDd6LpjV7SOnOH/3/VY6MUFQ0xaWHk9bwp/oBrDUZfSO8TzMwOOFFCSHJJGPp7ruXQy/erpTWM= X-Received: by 2002:a17:90a:d597:b0:28a:aa82:6ab3 with SMTP id v23-20020a17090ad59700b0028aaa826ab3mr1505790pju.31.1702365278663; Mon, 11 Dec 2023 23:14:38 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Andrew Pinski Date: Mon, 11 Dec 2023 23:14:27 -0800 Message-ID: Subject: Re: [PATCH] Treat "p" in asms as addressing VOIDmode To: Jeff Law , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-2.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, Dec 11, 2023 at 11:46=E2=80=AFAM Richard Sandiford wrote: > > Jeff Law writes: > > On 11/27/23 05:12, Richard Sandiford wrote: > >> check_asm_operands was inconsistent about how it handled "p" after > >> RA compared to before RA. Before RA it tested the address with a > >> void (unknown) memory mode: > >> > >> case CT_ADDRESS: > >> /* Every address operand can be reloaded to fit. */ > >> result =3D result || address_operand (op, VOIDmode); > >> break; > >> > >> After RA it deferred to constrain_operands, which used the mode > >> of the operand: > >> > >> if ((GET_MODE (op) =3D=3D VOIDmode > >> || SCALAR_INT_MODE_P (GET_MODE (op))) > >> && (strict <=3D 0 > >> || (strict_memory_address_p > >> (recog_data.operand_mode[opno], op)))) > >> win =3D true; > >> > >> Using the mode of the operand matches reload's behaviour: > >> > >> else if (insn_extra_address_constraint > >> (lookup_constraint (constraints[i]))) > >> { > >> address_operand_reloaded[i] > >> =3D find_reloads_address (recog_data.operand_mode[i], (rtx*) = 0, > >> recog_data.operand[i], > >> recog_data.operand_loc[i], > >> i, operand_type[i], ind_levels, insn)= ; > >> > >> It allowed the special predicate address_operand to be used, with the > >> mode of the operand being the mode of the addressed memory, rather tha= n > >> the mode of the address itself. For example, vax has: > >> > >> (define_insn "*movaddr" > >> [(set (match_operand:SI 0 "nonimmediate_operand" "=3Dg") > >> (match_operand:VAXfp 1 "address_operand" "p")) > >> (clobber (reg:CC VAX_PSL_REGNUM))] > >> "reload_completed" > >> "mova %a1,%0") > >> > >> where operand 1 is an SImode expression that can address memory of > >> mode VAXfp. GET_MODE (recog_data.operand[1]) is SImode (or VOIDmode), > >> but recog_data.operand_mode[1] is mode. > >> > >> But AFAICT, ira and lra (like pre-reload check_asm_operands) do not > >> do this, and instead pass VOIDmode. So I think this traditional use > >> of address_operand is effectively an old-reload-only feature. > >> > >> And it seems like no modern port cares. I think ports have generally > >> moved to using different address constraints instead, rather than > >> relying on "p" with different operand modes. Target-specific address > >> constraints post-date the code above. > >> > >> The big advantage of using different constraints is that it works > >> for asms too. And that (to finally get to the point) is the problem > >> fixed in this patch. For the aarch64 test: > >> > >> void f(char *p) { asm("prfm pldl1keep, %a0\n" :: "p" (p + 6)); } > >> > >> everything up to and including RA required the operand to be a > >> valid VOIDmode address. But post-RA check_asm_operands and > >> constrain_operands instead required it to be valid for > >> recog_data.operand_mode[0]. Since asms have no syntax for > >> specifying an operand mode that's separate from the operand itself, > >> operand_mode[0] is simply Pmode (i.e. DImode). > >> > >> This meant that we required one mode before RA and a different mode > >> after RA. On AArch64, VOIDmode is treated as a wildcard and so has a > >> more conservative/restricted range than DImode. So if a post-RA pass > >> tried to form a new address, it would use a laxer condition than the > >> pre-RA passes. > > This was initially a bit counter-intuitive, my first reaction was that = a > > wildcard mode is more general. And that's true, but it necessarily > > means the addresses accepted are more restrictive because any mode is > > allowed. > > Right. I should probably have a conservative, common subset. > > >> This happened with the late-combine pass that I posted in October: > >> https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634166.html > >> which in turn triggered an error from aarch64_print_operand_address. > >> > >> This patch takes the (hopefully) conservative fix of using VOIDmode fo= r > >> asms but continuing to use the operand mode for .md insns, so as not > >> to break ports that still use reload. > > Sadly I didn't get as far as I would have liked in removing reload, > > though we did get a handful of ports converted this cycle > > > >> > >> Fixing this made me realise that recog_level2 was doing duplicate > >> work for asms after RA. > >> > >> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install? > >> > >> Richard > >> > >> > >> gcc/ > >> * recog.cc (constrain_operands): Pass VOIDmode to > >> strict_memory_address_p for 'p' constraints in asms. > >> * rtl-ssa/changes.cc (recog_level2): Skip redundant constrain_ope= rands > >> for asms. > >> > >> gcc/testsuite/ > >> * gcc.target/aarch64/prfm_imm_offset_2.c: New test. > > It all seems a bit hackish. I don't think ports have had much success > > using 'p' through the decades. I think I generally ended up having to > > go with distinct constraints rather than relying on 'p'. > > > > OK for the trunk, but ewww. > > Thanks, pushed. And yeah, eww is fair. I'd be happy for this to become > an unconditional VOIDmode once reload is removed. The testcase prfm_imm_offset_2.c fails with a compile error: /home/apinski/src/upstream-full-cross/gcc/gcc/testsuite/gcc.target/aarch64/= prfm_imm_offset_2.c: In function 'f': /home/apinski/src/upstream-full-cross/gcc/gcc/testsuite/gcc.target/aarch64/= prfm_imm_offset_2.c:1:46: error: expected ')' before ':' token Most likely you need to add `/* { dg-options "" } */` to the beginning of the file so it does not compile with `-ansi -pedantic-errors` options which are default for the testsuite. Thanks, Andrew > > Richard