[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Andrew Pinski 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 ---
Confirmed looks like a target issue dealing with memcmp.
Take:
```
// $ cat bug.cpp
#include
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 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.
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
--- Comment #2 from Andrew Pinski ---
Here is a C testcase:
```
#include
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 ...
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598 Sergei Trofimovich changed: What|Removed |Added CC||roger at nextmovesoftware dot com --- Comment #3 from Sergei Trofimovich --- git bisect landed on the r14-2087-g83269719640689 : $ git bisect good 83269719640689415c0d5026ebfe05a0cf2bab72 is the first bad commit commit 83269719640689415c0d5026ebfe05a0cf2bab72 Author: Roger Sayle Date: Mon Jun 26 09:36:02 2023 +0100 i386: New *ashl_doubleword_highpart define_insn_and_split.
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598
Roger Sayle changed:
What|Removed |Added
Assignee|unassigned at gcc dot gnu.org |roger at
nextmovesoftware dot com
Status|NEW |ASSIGNED
--- Comment #4 from Roger Sayle ---
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.
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598 --- Comment #5 from CVS Commits --- The master branch has been updated by Roger Sayle : https://gcc.gnu.org/g:d2c18b4a16f9e1a6ed271ec1efaf94533d1c4a94 commit r14-2465-gd2c18b4a16f9e1a6ed271ec1efaf94533d1c4a94 Author: Roger Sayle 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 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.
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598 --- Comment #6 from Sergei Trofimovich --- The change fixed test suite on llvm-14.0.6 and on llvm-12.0.1. Thank you!
[Bug target/110598] [14 Regression] wrong code on llvm-14.0.6 due to memcmp being miscompiled
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110598 Roger Sayle changed: What|Removed |Added Resolution|--- |FIXED Status|ASSIGNED|RESOLVED Known to work||14.0 --- Comment #7 from Roger Sayle --- Many thanks to Sergei for confirming this issue is now resolved. Sorry again for the inconvenience.
