From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-db3eur04olkn0828.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe0c::828]) by sourceware.org (Postfix) with ESMTPS id 0628D3858D29 for ; Mon, 17 Jun 2024 05:07:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0628D3858D29 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=hotmail.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hotmail.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 0628D3858D29 Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=2a01:111:f400:fe0c::828 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1718600854; cv=pass; b=wJqlRaIT4XjbUw0kRAS3+1uwc0u5dGuvlt/BYE1DoL5Rg/O7d1GivVzOXkGSoukgStIxXspFwFF0xmzm/k97JtuarZ24hrimU5zGiY5hps1UxSloGyNdh5l+ul69g3qI/+YcNo2JiOEWsrYKZ10N+333E4HalXDilmFsrEWaiD8= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1718600854; c=relaxed/simple; bh=6zH4Y9a4sKpm8qFxa44FLIoYHrh/wDXdF71mVINcrB8=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=tfarORwVz/dJZw9nVq3yz/rije2lFGHZum+LavVInZ8xCrkyetsEhhcoGELyRrcnLFVTwtVt2+EmCqEXPpATDTdDA6P9cbFgyRNsP6J1AYk1XC7wrVhGPjZDw33LssA7qeitFBxeaXLhZ2TMDPrCJyhIWGTpxHecr3FSSDiRWLk= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=BIVhcBoNlZGbI7ts+a1lo6GHg4EAjCoarr5Vzuof5RlFJUowCcJjYfDEu59jPdpNljohwfGnhxOdA8RKK0Zp5wFJS2EWh0S6Ly+A1MKskrg6K4aHxtk5c4OfXiWCpS7s4dFSCvFNmNqlU9KKWec+GGcE0rw6049dUwIoU1+mj0Z6+ynjq4Hn8Sl8vubsGgOzCYCkftFM9k5JcbiQe2+MqbvBNNEZ2zy6ozErjUpmWomrC0K0M6wc5iPNwHPzq5k+xgqRT2Ly4aAy6ZQxwx2G2mHr67iR4jNjKm5T4jSXarmYphGM2T5ZrZ5tmZtncKbuqkyEHG8oe9lS6/04htGFlQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jFS7Pafy+S2UvkpwADBp5Y6e2N6ieCOslIiFcDfD09I=; b=k8w1cQ+m1Pow+b+OEboNA6RjiGep4wYYgb7kQKE8pacNX3+xHVgbZji5Y5P719K20DC6/isvrVPJngv9Mvo4qIv0ZquHCbhK4U9a4KzlJsumRqBvy4NkJCT2KU+9wd0sGGuCaHeanaaztbeNHFzuRKJiMVp0+yaVeFGqCq65J2ZoOJ5jOB10wFaLccWom8sGSa6msOjS7Xxl6lz8wF3bGhV5c/qpmjYtC4dUSgnA8lpSzLrBsP5OKfxmGb20UGUptAUtMeHA1jw4JPd2cCbIX7V35ru/Ly+POCtPqJy9US62NMPy5QY/RZGkwykbXBRKWZwmQHjHAPHydXtJhlKMoA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=HOTMAIL.DE; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=jFS7Pafy+S2UvkpwADBp5Y6e2N6ieCOslIiFcDfD09I=; b=f1s40E3amPw72drUpNAarUoS83e+w2Z/SvzTDXnxlLAA3frKgi0QNbPD0Yx51Qny8CgMJz5AyjFPbH3Yavp61/AVEpa5mG+mLIEFo049DshFQiUoWyL/IpMGkrHC6YmAK93AwP8cUVx/Wm9ewIGohPBPDYawRYaDnvsxgkw/5EaxrXzwOJGfTDwiOPJqtrqKRrbM4wr6X9a3Xh1O6u65rgF1NKt/t7UdsOUaJeFnyENtOjodKnbfym0h1bt+CcwrYZyHfzXIJeXkWmvuHSZxCylKMXfPBSPY6wv9LzHK5ZRhP0WY8vKdTVQBkm/0q0qMQ7RWbYzoWvkzS7w/qScJOA== Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) by AM8P193MB0802.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:1e1::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7677.30; Mon, 17 Jun 2024 05:07:29 +0000 Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::e65d:5dd2:5662:c61f]) by AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::e65d:5dd2:5662:c61f%4]) with mapi id 15.20.7677.030; Mon, 17 Jun 2024 05:07:29 +0000 Message-ID: Date: Mon, 17 Jun 2024 07:10:00 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 2/2] sim: riscv: Simplify the signed div by -1 code To: Andrew Burgess , "gdb-patches@sourceware.org" References: <87frtja1kr.fsf@redhat.com> Content-Language: en-US From: Bernd Edlinger In-Reply-To: <87frtja1kr.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-TMN: [c43LDj7zOAc9aPZfheKR8kVuE8/whfGDDiLJujHvgwVcSV0LK752a6/2kewRXRoc] X-ClientProxiedBy: BE1P281CA0159.DEUP281.PROD.OUTLOOK.COM (2603:10a6:b10:67::11) To AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) X-Microsoft-Original-Message-ID: <9866779a-3c31-4ca2-aee1-886a1ebba66d@hotmail.de> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P193MB1285:EE_|AM8P193MB0802:EE_ X-MS-Office365-Filtering-Correlation-Id: 50f700d3-84eb-4cf4-a061-08dc8e8b6316 X-Microsoft-Antispam: BCL:0;ARA:14566002|461199025|1602099009|4302099010|440099025|3412199022; X-Microsoft-Antispam-Message-Info: 3M1j17kF6NOIaEkq/WYosD+4qWJZx373ECt78h4TzFcuXhoHONzkU+jxRaeV3fC2jyQjIptALQK6jeTh0ls5hj9/J34+e+7QEQ/46Hk9+h8i0JpjxfZ++ePGWjDwH7qQZF6n+MVv6YInNhPW0yY2djWn+S+5G1tG8iMQ9wDPToLLJn2MAwKXnfU7wXtXiks89OSX0AwshifGIWtUfPYArioP4/SGp17ja20HCDyMsdt4Xf8nTPYK0mAyoWEJl8ilMDDXV/NMqLB8/CEZSxUPww01vMx29FBcycGCCtHKwksh81nmjq7fiGMFbtEy8WuhkYh2K9JCzYuUy96hNa6+/K6O5144HVnzRqK0jz57Ehben+7zDZTraxr7HhOjeOEZNLX3dgODt89RuSIQKe1ydsM+PlHZHjaMmrV9Kq4tNIyH7ABwv2GUi34diYpwok3IyUd6SuQgbY2fopr4AfVshGzZ8SUjpZf+Au0z8pzWQEJj979i3yAypzCNwx76xV0VPmGuqfaYO6SfZfR2dsbpicvuSvPn/IE7IBAbUFfeQtbFXpIjfXU8WSsThJy/kP0bN6qkVob58ag0j1fNJ80yJ4JtXgVYeZsglZQDEZ+yHMB0Cukp5VGleS/GGT6bKPjG6oPZDJ3OmU5RhcW0EPREACVoUxET+QhFXrLiN+VzO5YeOtCdFYVTDkwRnbJeZ79L X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ZzgvVWdMMjd1d0hxNDAzbVBnb3A0azE5emljOEI1aW1WQStXVVhaZG5HWGFr?= =?utf-8?B?bEc2T2JZb0VCaGIzUnBGM2VPODhpNTFCczVydHBqNGg0dkpmYk8zKy9aWVl1?= =?utf-8?B?RDlXUU1nc1pEblhVWXIwZ1BSYUxhUS9TZStqUmlreDhuWDN6N1FDWVdzYkUr?= =?utf-8?B?bWZNZkUzazJXNm5MZGNqdVBYZEwxNWRLNlVkOFdNbWFUWEV3ZnVzY1BlZk5m?= =?utf-8?B?TEJoaHl6UE9Nb1NEaDdaa21IMVprRXJkSnlkckZ1Q0FkRjY5MzNoREdTSzYx?= =?utf-8?B?WHE0N0ZnU3o5OXNnUFd2ZGJZMzZ1K1Y1c3BWekthOXZJbEhiOWlrcHNuMXFz?= =?utf-8?B?YWdheHRaTXhHN3d0SWJVaTNtNjZYMXdxSDVUM0R4a2JkOWpYMm1pS0ZLQnBE?= =?utf-8?B?LzBTVkx6MHBEc09OT2ZIMGlPQnI2dTUyMitHalZpdU0rN0xud0VEUEh4MzE4?= =?utf-8?B?dThoRlh6ZDF0U0Qxc0dmYTJnUVJsTloyVkVqcURrRnlkOXZhWEo3aWUyay9p?= =?utf-8?B?bGNuYkFDM0xOejBNQUlCT2VsaWpPd2Vud3hhenRRYWQwRlFkOWtwYTJaKytY?= =?utf-8?B?a2dBTklHc3lyUW5sSGxDcmZMSU8xaksyYnNlM1ZTcVQrZmF1WFVLdXVsaXB5?= =?utf-8?B?KzBSWTErSHRQR3p2VkNHczJyNFhaM3VpNC93Zm41Q0E5azRONUllcEU2NHpN?= =?utf-8?B?TFF3ZHpHOGtNaVlQOTZWbkpzWWo1Z24zNXBRd0c2ejZmd0xDa201c2lSajZ0?= =?utf-8?B?UUtxTkZYSnd0V0FTbG9uTGFtdUw2ZXJTQm1CaW0zQlExZ1ZsbjVLcXFRNHls?= =?utf-8?B?RkpoQXRTS0pBNzY3NUlGbExMNFNFWnRGS2VVb1JFQUFHKzRZSVBXZVJUL2s0?= =?utf-8?B?S2s0VnVKcCtYQUJtVlo0dVNjRkM4NklWU2c5amZkd2IwU2IyeG9sZEltUnU2?= =?utf-8?B?TXBTRDZPYkpOc3pkd2g4aHNvOTFCckRTbXRQRVZLODdjZEQvY0xRdGswbDVQ?= =?utf-8?B?Qk43OXF5bGJQdUw3OU1pTGh5WXZKTWVtTkUzdzlPY1ZHU3ZJa05UQnluYzRm?= =?utf-8?B?a1J5VnFLZzQycDM4dm1mMUpGcHNWZ2ZuOUxaYmFob09WR2ZtbVN2RllMQzA3?= =?utf-8?B?eEdTM3FsWnExMDJ4RWFaMlcxblo1QTJaVTYwUDM0TG1RWUNvV0xYdlFDS3Fh?= =?utf-8?B?a254Q3VNcFRaNDJQQTFGSjNCVXJ2QTQ1UGtzVEx0Sm1TOHFObEx6QWlFcEFD?= =?utf-8?B?emFub1N1NGM3L0lOdGwrbDdjeXJna29lSmROQ2Z4a3EzdDVMaHVRcGJhTzkw?= =?utf-8?B?TjQ3MnBZYlZwQkFBelNGd0NJQnZzbW1ESUtJdTBDdTRGaUlTbHVxZldWaDhH?= =?utf-8?B?eVczQWtQM0xLTDBMRXdtRitEaG44VzBGSWtOYUQ5TjdDUk5qa3UxZ09nL2cz?= =?utf-8?B?cGZxczE2SXB0VmdlZ2wwblNvSTh5UHlXUTIvWXRqSktrK2p3S0tHZFBKcGJz?= =?utf-8?B?K2tGL3Z1WTdaYW04Zm14UEpab0FuQmVaMm95aGcxRlVLbG1BanFLYUh6N05B?= =?utf-8?B?MVhhRFpYNmI0Ymx6V3VSdmwvU0ttWEFkbXo5TXloVXpBTElyMTJjY0p1SU1x?= =?utf-8?B?MDFvNEJDaHJMcWxhcVlHajV0ZG9uV1pyREUxblhvNzdhYXE0STZrMzFOZlVD?= =?utf-8?B?TFozaGhSbHUxU082TTF2aWhTV1Rodm1FbnRoKzZOd29qRlN5VFV5cmRzaUJx?= =?utf-8?Q?9wiT718OidDNbZyCVKwHXETJeipbxBIf5ZARoEP?= X-OriginatorOrg: sct-15-20-4755-11-msonline-outlook-80ceb.templateTenant X-MS-Exchange-CrossTenant-Network-Message-Id: 50f700d3-84eb-4cf4-a061-08dc8e8b6316 X-MS-Exchange-CrossTenant-AuthSource: AS8P193MB1285.EURP193.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jun 2024 05:07:29.6579 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8P193MB0802 X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,SPF_HELO_PASS,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 6/11/24 18:25, Andrew Burgess wrote: > Bernd Edlinger writes: > >> This uses the idea from the previous patch to >> simplify the code for non-overflowing signed >> divisions by -1. This is no bug-fix but it >> simplifies the code and avoids some unnecessary >> branches. >> --- >> sim/riscv/sim-main.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/sim/riscv/sim-main.c b/sim/riscv/sim-main.c >> index 515ff835223..e4b15b533ba 100644 >> --- a/sim/riscv/sim-main.c >> +++ b/sim/riscv/sim-main.c >> @@ -700,18 +700,16 @@ execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) >> const char *rd_name = riscv_gpr_names_abi[rd]; >> const char *rs1_name = riscv_gpr_names_abi[rs1]; >> const char *rs2_name = riscv_gpr_names_abi[rs2]; >> - unsigned_word tmp, dividend_max; >> + unsigned_word tmp; >> sim_cia pc = riscv_cpu->pc + 4; >> >> - dividend_max = -((unsigned_word) 1 << (WITH_TARGET_WORD_BITSIZE - 1)); > > I really don't understand what this was trying to calculate. It's not > the max value of an unsigned_word. Nor is it the bit representation of > the maximum signed_word value... but you're removing it, so I guess I > can not worry about that :) > This is the largest negative dividend, either -2**31=-2147483648 or -2**63=-9223372036854775808 and dividing this value by any non-negative value is always fine, except for the divisor=-1, because the result will overflow, and can therefore cause problems. But by my reasoning I found a simple solution that handles the divisor -1 for all dividends correctly and more efficiently, by simply computing -(unsigned)x instead of x/-1. >> - >> switch (op->match) >> { >> case MATCH_DIV: >> TRACE_INSN (cpu, "div %s, %s, %s; // %s = %s / %s", >> rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name); >> - if (riscv_cpu->regs[rs1] == dividend_max && riscv_cpu->regs[rs2] == -1) >> - tmp = dividend_max; >> + if (riscv_cpu->regs[rs2] == -1) >> + tmp = -riscv_cpu->regs[rs1]; > > As with the previous commit, can't we drop this whole if check and block? > No in this case the division would indeed overflow on the division dividend_max/-1. > I think we should add some tests for this too. The test I proposed in > the previous commit can be modified to give us div.s and rem.s tests. > Well, since this is just refactoring, and there is already very good test coverage in the gcc testsuite, I would not do that here, but instead I think the overflow bug in the mulh instruction really deserves a test case: [PATCH] sim: riscv: Fix undefined behaviour in mulh and similar https://sourceware.org/pipermail/gdb-patches/2024-April/208616.html So I will post an update for this one with a test case that demonstrates the possible mis-compilation due to a compiler optimization based on the assumption that an undefined behaviour may never happen, which caused user visible effects in this case. Thanks Bernd. > Thanks, > Andrew > >> else if (riscv_cpu->regs[rs2]) >> tmp = (signed_word) riscv_cpu->regs[rs1] / >> (signed_word) riscv_cpu->regs[rs2]; >> @@ -793,7 +791,7 @@ execute_m (SIM_CPU *cpu, unsigned_word iw, const struct riscv_opcode *op) >> case MATCH_REM: >> TRACE_INSN (cpu, "rem %s, %s, %s; // %s = %s %% %s", >> rd_name, rs1_name, rs2_name, rd_name, rs1_name, rs2_name); >> - if (riscv_cpu->regs[rs1] == dividend_max && riscv_cpu->regs[rs2] == -1) >> + if (riscv_cpu->regs[rs2] == -1) >> tmp = 0; >> else if (riscv_cpu->regs[rs2]) >> tmp = (signed_word) riscv_cpu->regs[rs1] >> -- >> 2.39.2 >