From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) by sourceware.org (Postfix) with ESMTPS id C92113858D35 for ; Tue, 23 Apr 2024 00:42:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C92113858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=dabbelt.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=dabbelt.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org C92113858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::62d ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713832961; cv=none; b=DizSA4XF/MS4Hqspq3CVdEJY/H4ZEp1y3Li3qrrrF2JjzfRBQDtXjxaVR61AgGpcoRceEaJF3dJaOGZ3jYT+upzX60bW9Qjd1Fw94GW66RmRTG2mz+y5vahGBrE8CFOFriEOEwL+t8bssRiV7jmf9KGvTFEvF0JHIGvV73jxIRY= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713832961; c=relaxed/simple; bh=fAjqL0wjS968O4I5a9Zh1aLVEVGWsMVpzMvD+7jvH8c=; h=DKIM-Signature:Date:Subject:From:To:Message-ID:Mime-Version; b=LhrVKfGBot8C8lEGph6FvBvIH8JoovZ5hUDm/yKhv/OzutOtmznnwzL8XyUVh5A3hh/o8CSpF+VU+e93hDI5Kh1PF7UJ+ggoE2F3Li7pzYwxUiHR2Qcr7aQHQT2fqZmc52fvsPtOjbF+yzOhRSs6Dtuuy5i09eTvykXgnRg0p5o= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pl1-x62d.google.com with SMTP id d9443c01a7336-1e2b137d666so39506905ad.2 for ; Mon, 22 Apr 2024 17:42:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dabbelt-com.20230601.gappssmtp.com; s=20230601; t=1713832958; x=1714437758; darn=gcc.gnu.org; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:from:to:cc:subject:date:message-id :reply-to; bh=d1g2e9xskBJc/35OBd+2U+nvBYk4sQpJaJrvPw5d03c=; b=HlbDsOqfd2fk1eIkYva2vdE5hYrp2Yss17PdEzceGhCdR8mIH/0FqURJ8pLexv/Kl/ k75d+0VudUeKYcP0T6zzGI5f03S7hhVa+cdYeZx0aMrTexdiiY79NorzrNsUk7Tt/msI BKJb/Ruw2uEGdQK4LxXb1OnfX/cZc2fH62Te3m7C5l0hajFaQUo1g5uM7CYlrCv8K/nD tzDogfPD+bxqgtoY35oEkc1g46BIT8jDjPTsxxbadJopQPSUEqY3Eb/mDFr4/WI+5enV +WfUjL4cVLVvuaRpoPZj7Vtng1uxbyKxxNArWtmCh0wlAKL657M3THNCqGb81DS3Y7Lf 74BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713832958; x=1714437758; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=d1g2e9xskBJc/35OBd+2U+nvBYk4sQpJaJrvPw5d03c=; b=ah1iVqFsdTwafRumsJkAAC+grrn1SlsLGZzpR7D6eQULlPcQ5qaLoiBGi+16lekkEk CCoaKycAP9RqckrreTCjgCGKTrYbaplZ8kHcb8In+hs08+LVDPTahmJcc05uYRsu/svA zBdv5Jg2RneIo2B7jjtDmPcxgYMCJeG16FVFhYlX1dMlOMIEGeXOikNoNzM6xPmseCGQ +L4OtsTaIeVsnEWRYmPG+z0hOBWd6z/BhvINIX7BXkQug3IqL4V7JKxD8qleewT0nZ8Y 3PkT0vCtcBVNVfSYazLt+RyhlAYrhJbI+JazpvUoIwVXC7xvUG9G298tj5PXusy4oTip 8wVg== X-Forwarded-Encrypted: i=1; AJvYcCVi0B7kZw2C18dxkAg45y0zqjpOdiFCXQ6xMn33YuxO/RHoz7twAr9Y2Q54vVildStireYlTCKNBsPRC4twgTQN6HS/Uqyqxw== X-Gm-Message-State: AOJu0Yx+ugm66yKkRkBfizFiNuFPOQ/nsDI5zIlYV6wVfs7pCySF4TQx fDZBfc/1SOfMEmjJErSTJDvBUWbbPVb4mfHweCl9lrxB88+lLdBD54T31l5EY0c= X-Google-Smtp-Source: AGHT+IG2QNJn8T3CHe9MyOVubBjUccZEgzT7FMZpidPFUHgwJD+lkYQjeS6XrwmTZBDvqT9toaXVcw== X-Received: by 2002:a17:902:ec8a:b0:1e9:a0ce:f60e with SMTP id x10-20020a170902ec8a00b001e9a0cef60emr7091503plg.19.1713832957514; Mon, 22 Apr 2024 17:42:37 -0700 (PDT) Received: from localhost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id h10-20020a170902b94a00b001e088a9e2bcsm8734700pls.292.2024.04.22.17.42.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 22 Apr 2024 17:42:36 -0700 (PDT) Date: Mon, 22 Apr 2024 17:42:36 -0700 (PDT) X-Google-Original-Date: Mon, 22 Apr 2024 17:42:33 PDT (-0700) Subject: Re: Re: [PATCH v1] RISC-V: Revert RVV wv instructions overlap and xfail tests In-Reply-To: <4987EAADD3EAF406+2024042306075876990914@rivai.ai> CC: Patrick O'Neill , pan2.li@intel.com, Robin Dapp , gcc-patches@gcc.gnu.org, Kito Cheng From: Palmer Dabbelt To: juzhe.zhong@rivai.ai Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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, 22 Apr 2024 15:07:59 PDT (-0700), juzhe.zhong@rivai.ai wrote: > Apologize that we didn't post our (me, kito and Li Pan) disscussions. Some amount of off-list discussion is inevitable, but as far as I can tell here we ended up with some code committed that wasn't even posted to the lists and didn't even build. I don't know exactly where the bar for public discussions is, but it's got to be somewhere higher than that. > This is the story: > We found that my previous patches which support highpart register overlap with register filter for instructions like (vwadd.wv) > cause ICE reported by: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114714 > and this is obviously a regression (No ICE on GCC 13.2, but ICE on GCC 14) > > We have tried several fixes to work around this ICE, however, we failed. > And also I found my previous patches are quite wrong which is not the perfect solution to support register group overlap > for vwadd.wv. > So, finally we decide to revert those patches. Sure, reverting something that has a bug is reasonable. The problem is how this happened: this code clearly did not get tested, as it doesn't even build and re-introduces a bunch of ICEs. We're very late in stage 4 and this is the second time the entire port has been broken in as many weeks. That's a really bad time to be breaking things. I think we're really set the wrong precedent on what the bar is for review here. We had a lot of breakages early on in the 14 development cycle and never really dug into that, I was hoping that getting CI set up would be a strong enough hint to stop the breakages. Clearly that didn't work, though, so: Please stop breaking the port. It's exceedingly rare that any patch needs to be committed minutes after it's posted. These port-wide breakages are really the only thing where it could be agreed that's the way to go, but as we can see here rushing is as likely to dig a bigger hole as it is to fix things. I get the testsuite can be kind of hard to run, but if you don't want to run it locally you can just wait for the CI to do it for you. That's not really asking for very much. > Kito knows the details of this story, kito can share more details in GNU patche meeting. Ya, we can talk tomorrow morning. Do you guys have a fix for the regressions that showed up over the weekend? Either way I'd prefer to go with reverting all this and then taking Robin's more self-contained fix. If you guys want to do a bigger change later that's fine, we're just really close to the release and it's not a good time to risk breaking things. We've only had a few days of a functioning port over the last week or two, that's already put us behind on the distro prerelases/RCs so I'm kind of worried something else has slipped in. > > Thanks. > > > juzhe.zhong@rivai.ai > > From: Patrick O'Neill > Date: 2024-04-23 01:20 > To: Li, Pan2; Robin Dapp; gcc-patches@gcc.gnu.org > CC: juzhe.zhong@rivai.ai; kito.cheng@gmail.com > Subject: Re: [PATCH v1] RISC-V: Revert RVV wv instructions overlap and xfail tests > Hi Pan, > I'm not sure I'm following. Did we miss something that should have been > covered? Like only an overlap on the srcs but not the dest? > Are there testcases that fail? If so we should definitely have one. > Can you give some additional information on why these reverts are needed? > +1 to the request for a failing testcase if there is one. Patrick If something is broken then indeed we should revert it. > Yes, we may need to support these in gcc-15. > ... why not just revert everything and xfail all the tests in a > follow up? Your patch is essentially a revert but doesn't look like > it. I'd rather we let a revert be a revert and adjust the tests > separately so it becomes clear. > Sure, will revert b3b2799b872 and then file the patch for the xfail tests. > Pan >