From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id E39B43858D28 for ; Thu, 29 Sep 2022 06:16:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E39B43858D28 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linux.ibm.com Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 28T5vq3S026927; Thu, 29 Sep 2022 06:16:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=GUvQ2rpw/u+SvpYEhbvr3KbSx5OlGC08cvuMtVwLH2I=; b=DXE8inoGIufXZuhfceX74F0Sl/Q9MZndgGDOA9+v7Lr0i8FEvbuVbyBme3EnrQyoD+yb NZskED/W+JD0+puj/Jyi7PdCcmXjD1FD2BqmI2Eq7esto9mEJAQo+hR3nBJCFPt6Ujks JnbFy3YDP+5XQkxwWI7URYbp+cAEXxlrpj+WMXqPRXnuLh4jsoVlndBecrR9+W1DHPg+ oUAG1jx6d2FBKR1uR9ZloUK3ySW1WAWU2iPPZw2/ZTxlSuMUN3k5pqihkQIaIb9onp18 8DmTydQ5bkYTtOJQHB+gVLQCBv0MoDFatttzlspYgJfyXdmiVVXbfydz1tBQqpQiDvuj SQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3jw5p78f3k-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Sep 2022 06:16:13 +0000 Received: from m0098399.ppops.net (m0098399.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 28T5wopr028966; Thu, 29 Sep 2022 06:16:12 GMT Received: from ppma05fra.de.ibm.com (6c.4a.5195.ip4.static.sl-reverse.com [149.81.74.108]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3jw5p78f1v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Sep 2022 06:16:12 +0000 Received: from pps.filterd (ppma05fra.de.ibm.com [127.0.0.1]) by ppma05fra.de.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 28T66HpG025590; Thu, 29 Sep 2022 06:16:10 GMT Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by ppma05fra.de.ibm.com with ESMTP id 3jssh8vmqa-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 29 Sep 2022 06:16:10 +0000 Received: from d06av21.portsmouth.uk.ibm.com (d06av21.portsmouth.uk.ibm.com [9.149.105.232]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 28T6G8SW5440056 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 29 Sep 2022 06:16:08 GMT Received: from d06av21.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E39EA52050; Thu, 29 Sep 2022 06:16:07 +0000 (GMT) Received: from [9.197.253.23] (unknown [9.197.253.23]) by d06av21.portsmouth.uk.ibm.com (Postfix) with ESMTP id 25F7B5204E; Thu, 29 Sep 2022 06:16:05 +0000 (GMT) Message-ID: Date: Thu, 29 Sep 2022 14:16:04 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH] rs6000: Rework option -mpowerpc64 handling [PR106680] Content-Language: en-US To: Segher Boessenkool Cc: GCC Patches , David Edelsohn , Peter Bergner , iain@sandoe.co.uk References: <9d9f1f43-b528-387d-45a7-1d89400de0fc@linux.ibm.com> <20220928220443.GV25951@gate.crashing.org> From: "Kewen.Lin" In-Reply-To: <20220928220443.GV25951@gate.crashing.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: bmBk3GQhKGpEd33VxlxLv6tI2afW-bNX X-Proofpoint-ORIG-GUID: Db-vIOZQqbciIseV5bZG6_AZjecPy9xS X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.528,FMLib:17.11.122.1 definitions=2022-09-29_04,2022-09-29_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 impostorscore=0 bulkscore=0 adultscore=0 clxscore=1015 suspectscore=0 lowpriorityscore=0 mlxscore=0 mlxlogscore=999 malwarescore=0 phishscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2209130000 definitions=main-2209290034 X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,KAM_SHORT,NICE_REPLY_A,RCVD_IN_MSPIKE_H2,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: Hi Segher! Thanks for the review comments!! on 2022/9/29 06:04, Segher Boessenkool wrote: > On Wed, Sep 28, 2022 at 01:30:46PM +0800, Kewen.Lin wrote: >> PR106680 shows that -m32 -mpowerpc64 is different from >> -mpowerpc64 -m32, this is determined by the way how we >> handle option powerpc64 in rs6000_handle_option. >> >> Segher pointed out this difference should be taken as >> a bug and we should ensure that option powerpc64 is >> independent of -m32/-m64. So this patch removes the >> handlings in rs6000_handle_option and add some necessary >> supports in rs6000_option_override_internal instead. > > Thanks! > >> With this patch, if users specify -m{no-,}powerpc64, the >> specified value is honoured, otherwise, for 64bit it >> always enables OPTION_MASK_POWERPC64 while for 32bit >> it disables OPTION_MASK_POWERPC64 if OS_MISSING_POWERPC64. > > If the user says -m64 -mno-powerpc64 it should error, and perhaps -m32 > -mpowerpc64 should warn if OS_MISSING_POWERPC64? OK ... > >> - /* Some OSs don't support saving the high part of 64-bit registers on context >> - switch. Other OSs don't support saving Altivec registers. On those OSs, >> - we don't touch the OPTION_MASK_POWERPC64 or OPTION_MASK_ALTIVEC settings; >> - if the user wants either, the user must explicitly specify them and we >> - won't interfere with the user's specification. */ >> + /* Some OSs don't support saving Altivec registers. On those OSs, we don't >> + touch the OPTION_MASK_POWERPC64 or OPTION_MASK_ALTIVEC settings; if the >> + user wants either, the user must explicitly specify them and we won't >> + interfere with the user's specification. */ >> >> set_masks = POWERPC_MASKS; >> -#ifdef OS_MISSING_POWERPC64 >> - if (OS_MISSING_POWERPC64) >> - set_masks &= ~OPTION_MASK_POWERPC64; >> -#endif > > As I said elsewhere, it probably is helpful if we still warn here for > -m32 -mpowerpc64 with OS_MISSING_POWERPC64 (or without the -m32 even, > same thing). > OK ... >> + /* With option powerpc64 specified explicitly (either on or off), even if >> + being compiled for 64 bit we don't need to check if it's disabled here, >> + since subtargets will check and raise an error message if necessary >> + later. But without option powerpc64 specified explicitly, we need to >> + ensure powerpc64 enabled for 64 bit and disabled on those OSes with >> + OS_MISSING_POWERPC64, since they don't support saving the high part of >> + 64-bit registers on context switch. */ >> + if (!(rs6000_isa_flags_explicit & OPTION_MASK_POWERPC64)) >> + { >> + if (TARGET_64BIT) >> + /* Make sure we always enable it by default for 64 bit. */ >> + rs6000_isa_flags |= OPTION_MASK_POWERPC64; >> +#ifdef OS_MISSING_POWERPC64 >> + else if (OS_MISSING_POWERPC64) >> + /* It's unexpected to have OPTION_MASK_POWERPC64 on for OSes which >> + miss powerpc64 support, so disable it. */ >> + rs6000_isa_flags &= ~OPTION_MASK_POWERPC64; >> +#endif >> + } > > Aha. Please don't, just warn instead? Silently disabling such stuff is > the worst option :-( ... I'll update this to warn instead as you suggested. :) > >> +/* { dg-error "'-m64' requires a PowerPC64 cpu" "PR106680" { target powerpc*-*-linux* powerpc-*-rtems* } 0 } */ > > Everything except AIX even? So it will include Darwin as well (and the > BSDs, and powerpc*-elf, etc.) I found this message only existed in file rtems.h and function rs6000_linux64_override_options, the latter is used by files linux64.h and freebsd64.h, I guess we just want to add one more powerpc*-*-freebsd*, but leave the others alone (and update this as needed later)? > >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/powerpc/pr106680-4.c >> @@ -0,0 +1,16 @@ >> +/* Skip this on aix, otherwise it emits the error message like "64-bit >> + computation with 32-bit addressing not yet supported" on aix. */ >> +/* { dg-skip-if "" { powerpc*-*-aix* } } */ >> +/* { dg-require-effective-target ilp32 } */ >> +/* { dg-options "-mpowerpc64 -m32 -O2" } */ > > If you have -m32 you don't need ilp32, and the other way around. > Will update! I was afraid the dejagnu version mattered, it can be: "-mpowerpc64 -m32 -O2 -m64" or "-m64 -mpowerpc64 -m32 -O2", but just realized -mpowerpc64 would always take effect, useless worry. :) >> +/* Verify option -m32 doesn't override option -mpowerpc64. >> + If option -mpowerpc64 gets overridden, the assembly would >> + end up with addc and adde. */ >> +/* { dg-final { scan-assembler-not "addc" } } */ >> +/* { dg-final { scan-assembler-not "adde" } } */ > > Lol, nice :-) > > "adde" is a frequent substring, use \m \M please? You will always get > these exact insns anyway. And you could add a -times {\madd\M} 1 ? Will update, thanks again for all the comments! > > The Darwin problem might be something in darwin*.h, but I don't see it. > Maybe it is a more generic problem? > Yeah, it's probably a generic problem but only got exposed on darwin, I just made a trial diff, hope it can work. BR, Kewen