From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x931.google.com (mail-ua1-x931.google.com [IPv6:2607:f8b0:4864:20::931]) by sourceware.org (Postfix) with ESMTPS id CB8B5384400A for ; Wed, 28 Oct 2020 01:30:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CB8B5384400A Received: by mail-ua1-x931.google.com with SMTP id r9so1067315uat.12 for ; Tue, 27 Oct 2020 18:30:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=8tWUMfRTqb8DWaN/N67a/Xs77VRj8P/HggONGP7I7gI=; b=ZX5VLc6V/Ik090Z+dMomQK/O9QEp0COzYHvzI91XLn0kf8fBT0njQYKRcoxaNaHgRM IbE+sth71c6ggKBG0/QsrgY9S947Kxp9vH+EU3gHMx6tqll9SVv7KGElf/4owGHdbVLR fj1ka2anLShshu03sRDoWC/KafqhvSZ1eWwOXXB+5oCRud/vxjnNBXRCD9+2a/Y6ut68 KTyUxFPCl3TUXzh5WdvTxDaxj1AGmzgOHSuq2sRGS9tuXM2bdzFV+jnQhKXMV28duzDe Bo1n136nWyFVLQRkmucizsKoqYLZNfiJY96RIqM92hPOSKZUyV2mpUNDqTFGzoy1v55Y 3zKg== X-Gm-Message-State: AOAM533lgPgEXrXfyxHWWZzl6HxFL0cd+NraifiORlpIycwnlgr9aOmq LQmYKYDKOXljl4IuOpdpCNDds8f8DnzqPjyLSx04in8q X-Google-Smtp-Source: ABdhPJxdD8MFQxXcgniz9l7uPxbK7+szset/Z5tPOHdcpati9egSJSphoCmhtjFD6dJ38LFel+fiq/3xKCxU1IDiI3A= X-Received: by 2002:a9f:37a8:: with SMTP id q37mr3148846uaq.102.1603848628223; Tue, 27 Oct 2020 18:30:28 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Hongtao Liu Date: Wed, 28 Oct 2020 09:32:05 +0800 Message-ID: Subject: Re: [PATCH][PR target/97540] Don't extract memory from operand for normal memory constraint. To: Hongtao Liu via Gcc-patches , Vladimir Makarov , Hongtao Liu , dcb314@hotmail.com, Richard Sandiford Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 28 Oct 2020 01:30:30 -0000 On Tue, Oct 27, 2020 at 7:13 PM Richard Sandiford wrote: > > Hongtao Liu via Gcc-patches writes: > > Hi: > > For inline asm, there could be an operand like (not (mem:)), it's > > not a valid operand for normal memory constraint. > > Bootstrap is ok, regression test is ok for make check > > RUNTESTFLAGS="--target_board='unix{-m32,}'" > > > > gcc/ChangeLog > > PR target/97540 > > * ira.c: (ira_setup_alts): Extract memory from operand only > > for special memory constraint. > > * recog.c (asm_operand_ok): Ditto. > > * lra-constraints.c (process_alt_operands): MEM_P is > > required for normal memory constraint. > > > > gcc/testsuite/ChangeLog > > * gcc.target/i386/pr97540.c: New test. > > Sorry to stick my oar in, but I think we should reconsider the > bcst_mem_operand approach. It seems like these patches (and the > previous one) are fighting against the principle that operands > cannot be arbitrary expressions. > > This kind of thing was attempted long ago (even before my time!) > for SIGN_EXTEND on MIPS. It ended up causing more problems than > it solved and in the end it had to be taken out. I'm worried that > we might end up going through the same cycle again. > > Also, this LRA code is extremely performance-sensitive in terms > of compile time: it's often at the top or near the top of the profile. > So adding calls to new functions like extract_mem_from_operand for > a fairly niche case probably isn't a good trade-off. > > I think we should instead find a nice(?) syntax for generating separate > patterns for the two bcst_vector_operand alternatives from a single > .md pattern. That would fit the existing model much more closely. > We have define_subst for RTL template transformations, but it's not suitable for this case(too many define_subst templates need to be added, and it doesn't show any advantage compared to adding separate bcst patterns.). I don't find other workable existing syntax for it. So suppose I should revert my former 2 patches and add separate bcst patterns. > (To be clear, I'm not saying the existing model is perfect. > I just think a change along these lines is more fundamental > than it might look, and would need changes outside the register > allocators to work reliably.) > > FWIW, in: > > (define_insn "*3" > [(set (match_operand:VI_AVX2 0 "register_operand" "=x,v") > (plusminus:VI_AVX2 > (match_operand:VI_AVX2 1 "bcst_vector_operand" "0,v") > (match_operand:VI_AVX2 2 "bcst_vector_operand" "xBm,vmBr")))] > > we can assume that any bcst_mem_operand will be first. Allowing it > as operand 2 (as the constraints do) creates non-canonical RTL. > So this at least is one case in which I think the bcst_mem_operand > version has to be a separate .md construct. > > Sorry for not noticing or speaking up earlier. I realise it's > extremely unhelpful to get this kind of comment after you've done > so much work. :-( > > Thanks, > Richard -- BR, Hongtao