* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
@ 2023-07-08 17:32 ` pinskia at gcc dot gnu.org
2023-07-08 17:41 ` pinskia at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-07-08 17:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2023-07-08
Status|UNCONFIRMED |NEW
Target Milestone|--- |14.0
Keywords| |wrong-code
Summary|[14 Regression] wrong code |[14 Regression] wrong code
|on llvm-14.0.6 |on llvm-14.0.6 due to
| |memcmp being miscompiled
Ever confirmed|0 |1
Component|c++ |target
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed looks like a target issue dealing with memcmp.
Take:
```
// $ cat bug.cpp
#include <iterator>
typedef unsigned long long u64;
const unsigned MAX_SUBTARGET_WORDS = 4;
[[gnu::noipa]]
int notequal(const void *a, const void *b)
{
return __builtin_memcmp(a,b,MAX_SUBTARGET_WORDS*sizeof(u64)) != 0;
}
struct FeatureBitset {
u64 Bits[MAX_SUBTARGET_WORDS] = {0, 0, 0, 0};
constexpr FeatureBitset() = default;
constexpr FeatureBitset(std::initializer_list<unsigned> Init) {
for (auto I : Init) {
Bits[I] = 1;
}
}
constexpr FeatureBitset operator&(const FeatureBitset &RHS) const {
FeatureBitset Result = *this;
for (unsigned I = 0, E = MAX_SUBTARGET_WORDS; I != E; ++I) {
Result.Bits[I] &= RHS.Bits[I];
}
return Result;
}
bool operator!=(const FeatureBitset &RHS) const {
return notequal(Bits, RHS.Bits);
}
};
static constexpr const FeatureBitset TargetFeatures = { 0, 0, 0, 0, };
__attribute__((noipa))
bool is_eq_buggy (const FeatureBitset & lf, const FeatureBitset & rf) {
if ((lf & TargetFeatures) != (rf & TargetFeatures))
return false;
return true;
}
__attribute__((noipa))
void bug(void) {
FeatureBitset lf, rf;
lf.Bits[0] = rf.Bits[0] = 1;
lf.Bits[1] = rf.Bits[1] = 1;
lf.Bits[2] = rf.Bits[2] = 1;
lf.Bits[3] = rf.Bits[3] = 1;
bool r = is_eq_buggy (lf, rf);
if (!r) __builtin_trap();
}
__attribute__((noipa))
int main(void) {
bug();
}
```
The above is not miscompiled but once the gnu::noipa on the notequal is
removed, it is.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
2023-07-08 17:32 ` [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled pinskia at gcc dot gnu.org
@ 2023-07-08 17:41 ` pinskia at gcc dot gnu.org
2023-07-08 20:31 ` slyfox at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-07-08 17:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Here is a C testcase:
```
#include <stdbool.h>
typedef unsigned long long u64;
#define MAX_SUBTARGET_WORDS 4
//[[gnu::noipa]]
int notequal(const void *a, const void *b)
{
return __builtin_memcmp(a,b,MAX_SUBTARGET_WORDS*sizeof(u64)) != 0;
}
typedef struct FeatureBitset {
u64 Bits[MAX_SUBTARGET_WORDS];
}FeatureBitset;
__attribute__((noipa))
bool is_eq_buggy (const FeatureBitset * lf, const FeatureBitset * rf) {
u64 Bits_l[MAX_SUBTARGET_WORDS];
Bits_l[0] = lf->Bits[0]&1;
Bits_l[1] = 0;
Bits_l[2] = 0;
Bits_l[3] = 0;
u64 Bits_r[MAX_SUBTARGET_WORDS];
Bits_r[0] = rf->Bits[0]&1;
Bits_r[1] = 0;
Bits_r[2] = 0;
Bits_r[3] = 0;
return !notequal(Bits_l, Bits_r);
}
__attribute__((noipa))
void bug(void) {
FeatureBitset lf, rf;
lf.Bits[0] = rf.Bits[0] = 1;
lf.Bits[1] = rf.Bits[1] = 1;
lf.Bits[2] = rf.Bits[2] = 1;
lf.Bits[3] = rf.Bits[3] = 1;
bool r = is_eq_buggy (&lf, &rf);
if (!r) __builtin_trap();
}
__attribute__((noipa))
int main(void) {
bug();
}
```
Again uncomment the attribute on notequal, if you want to see it works without
inlining of memcmp here ...
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
2023-07-08 17:32 ` [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled pinskia at gcc dot gnu.org
2023-07-08 17:41 ` pinskia at gcc dot gnu.org
@ 2023-07-08 20:31 ` slyfox at gcc dot gnu.org
2023-07-09 22:34 ` roger at nextmovesoftware dot com
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: slyfox at gcc dot gnu.org @ 2023-07-08 20:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Sergei Trofimovich <slyfox at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |roger at nextmovesoftware dot com
--- Comment #3 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
git bisect landed on the r14-2087-g83269719640689 :
$ git bisect good
83269719640689415c0d5026ebfe05a0cf2bab72 is the first bad commit
commit 83269719640689415c0d5026ebfe05a0cf2bab72
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Mon Jun 26 09:36:02 2023 +0100
i386: New *ashl<dwi3>_doubleword_highpart define_insn_and_split.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
` (2 preceding siblings ...)
2023-07-08 20:31 ` slyfox at gcc dot gnu.org
@ 2023-07-09 22:34 ` roger at nextmovesoftware dot com
2023-07-12 13:13 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: roger at nextmovesoftware dot com @ 2023-07-09 22:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |roger at nextmovesoftware dot com
Status|NEW |ASSIGNED
--- Comment #4 from Roger Sayle <roger at nextmovesoftware dot com> ---
Mine. The new peephole2 is misbehaving with the (odd) RTL sequence:
(insn 62 61 63 2 (set (reg:DI 1 dx [111])
(const_int 0 [0])) "pr110598.c":10:10 90 {*movdi_internal}
(insn 63 62 64 2 (parallel [
(set (reg:DI 1 dx [110])
(xor:DI (reg:DI 1 dx [111])
(reg:DI 1 dx)))
(clobber (reg:CC 17 flags))
]) "pr110598.c":10:10 626 {*xordi_1}
(expr_list:REG_UNUSED (reg:CC 17 flags)
(nil)))
Doh! I need to confirm/check that rega != regb. Interesting that DF doesn't
consider insn 62 as REG_UNUSED (i.e. dead), a classic false dependency.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
` (3 preceding siblings ...)
2023-07-09 22:34 ` roger at nextmovesoftware dot com
@ 2023-07-12 13:13 ` cvs-commit at gcc dot gnu.org
2023-07-12 16:15 ` slyfox at gcc dot gnu.org
2023-07-12 19:42 ` roger at nextmovesoftware dot com
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-07-12 13:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:
https://gcc.gnu.org/g:d2c18b4a16f9e1a6ed271ec1efaf94533d1c4a94
commit r14-2465-gd2c18b4a16f9e1a6ed271ec1efaf94533d1c4a94
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Wed Jul 12 14:12:34 2023 +0100
PR target/110598: Fix rega = 0; rega ^= rega regression in i386.md
This patch fixes the regression PR target/110598 caused by my recent
addition of a peephole2. The intention of that optimization was to
simplify zeroing a register, followed by an IOR, XOR or PLUS operation
on it into a move, or as described in the comment:
;; Peephole2 rega = 0; rega op= regb into rega = regb.
The issue is that I'd failed to consider the (rare and unusual) case,
where regb is rega, where the transformation leads to the incorrect
"rega = rega", when it should be "rega = 0". The minimal fix is to
add a !reg_mentioned_p check to the recent peephole2.
In addition to resolving the regression, I've added a second peephole2
to optimize the problematic case above, which contains a false
dependency and is therefore tricky to optimize elsewhere. This is an
improvement over GCC 13, for example, that generates the redundant:
xorl %edx, %edx
xorq %rdx, %rdx
2023-07-12 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
PR target/110598
* config/i386/i386.md (peephole2): Check !reg_mentioned_p when
optimizing rega = 0; rega op= regb for op in [XOR,IOR,PLUS].
(peephole2): Simplify rega = 0; rega op= rega cases.
gcc/testsuite/ChangeLog
PR target/110598
* gcc.target/i386/pr110598.c: New test case.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
` (4 preceding siblings ...)
2023-07-12 13:13 ` cvs-commit at gcc dot gnu.org
@ 2023-07-12 16:15 ` slyfox at gcc dot gnu.org
2023-07-12 19:42 ` roger at nextmovesoftware dot com
6 siblings, 0 replies; 8+ messages in thread
From: slyfox at gcc dot gnu.org @ 2023-07-12 16:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
--- Comment #6 from Sergei Trofimovich <slyfox at gcc dot gnu.org> ---
The change fixed test suite on llvm-14.0.6 and on llvm-12.0.1. Thank you!
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
2023-07-08 17:00 [Bug c++/110598] New: [14 Regression] wrong code on llvm-14.0.6 slyfox at gcc dot gnu.org
` (5 preceding siblings ...)
2023-07-12 16:15 ` slyfox at gcc dot gnu.org
@ 2023-07-12 19:42 ` roger at nextmovesoftware dot com
6 siblings, 0 replies; 8+ messages in thread
From: roger at nextmovesoftware dot com @ 2023-07-12 19:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
Known to work| |14.0
--- Comment #7 from Roger Sayle <roger at nextmovesoftware dot com> ---
Many thanks to Sergei for confirming this issue is now resolved. Sorry again
for the inconvenience.
^ permalink raw reply [flat|nested] 8+ messages in thread