public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug c++/104243] New: Optimization requires __sync_synchronize
@ 2022-01-26 14:37 aklitzing at gmail dot com
2022-01-26 15:07 ` [Bug c++/104243] " rguenth at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: aklitzing at gmail dot com @ 2022-01-26 14:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
Bug ID: 104243
Summary: Optimization requires __sync_synchronize
Product: gcc
Version: 6.4.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: c++
Assignee: unassigned at gcc dot gnu.org
Reporter: aklitzing at gmail dot com
Target Milestone: ---
Hi,
long time ago I found a strange behaviour with Qt and GCC 6.3. I tried it again
today with GCC 11.1.0.
If I compile this snippet with an optimization level it will output "broken".
If I use -O0 or use clang it will output "ok".
If I add "__sync_synchronize" it will work with any optimization, too. The
variable "idx" isn't updated in if-clause but it seems updated if qDebug is
called.
#include <QVersionNumber>
#include <QDebug>
int main(int argc, char** argv)
{
const QString pVersion = QStringLiteral("1.12.2");
int idx = 0;
const auto& version = QVersionNumber::fromString(pVersion, &idx);
// #ifdef Q_CC_GNU
// __sync_synchronize();
// #endif
if(idx == 0)
qDebug() << "broken";
else
qDebug() << "ok";
qDebug() << "idx:" << idx;
return 0;
}
$ g++ -O2 example.cpp `pkg-config --libs --cflags Qt5Core`
$ ./a.out
broken
idx: 6
$ g++ -O0 bla.cpp `pkg-config --libs --cflags Qt5Core`
$ ./a.out
ok
idx: 6
Source of QVersionNumber:
https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qversionnumber.cpp?h=5.15.2#n463
Qt-Report:
https://bugreports.qt.io/browse/QTBUG-62185
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
@ 2022-01-26 15:07 ` rguenth at gcc dot gnu.org
2022-01-26 15:10 ` marxin at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-01-26 15:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |WAITING
Last reconfirmed| |2022-01-26
Ever confirmed|0 |1
--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Please provide preprocessed source, QVersionNumber doesn't seem to be available
for me.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
2022-01-26 15:07 ` [Bug c++/104243] " rguenth at gcc dot gnu.org
@ 2022-01-26 15:10 ` marxin at gcc dot gnu.org
2022-01-26 15:10 ` marxin at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-01-26 15:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |marxin at gcc dot gnu.org
--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
It's related to fact that the following function is pure:
[[nodiscard]] __attribute__((visibility("default"))) static
__attribute__((pure)) QVersionNumber fromString(const QString &string, int
*suffixIndex = nullptr);
[[nodiscard]] __attribute__((visibility("default"))) static
__attribute__((pure)) QVersionNumber fromString(QLatin1String string, int
*suffixIndex = nullptr);
[[nodiscard]] __attribute__((visibility("default"))) static
__attribute__((pure)) QVersionNumber fromString(QStringView string, int
*suffixIndex = nullptr);
and thus we optimize out: qt.ii.037t.fre1
Replaced idx with 0 in all uses of idx.0_1 = idx;
Removing unexecutable edge from if (idx.0_1 == 0)
Removing dead stmt idx.0_1 = idx;
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
2022-01-26 15:07 ` [Bug c++/104243] " rguenth at gcc dot gnu.org
2022-01-26 15:10 ` marxin at gcc dot gnu.org
@ 2022-01-26 15:10 ` marxin at gcc dot gnu.org
2022-01-26 15:14 ` marxin at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-01-26 15:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Thus I think the bug tens to be invalid..
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (2 preceding siblings ...)
2022-01-26 15:10 ` marxin at gcc dot gnu.org
@ 2022-01-26 15:14 ` marxin at gcc dot gnu.org
2022-01-26 15:19 ` aklitzing at gmail dot com
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-01-26 15:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |INVALID
Status|WAITING |RESOLVED
--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
Simpler reproducer:
$ cat repro.cc
int
__attribute__((pure))
foo(int *x);
int main()
{
int a;
int r = foo (&a);
if (a != 123)
__builtin_abort ();
}
$ cat repro2.cc
int
__attribute__((pure))
foo(int *x)
{
*x = 123;
return 0;
}
$ gcc repro*.cc -O2 && ./a.out
Aborted (core dumped)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (3 preceding siblings ...)
2022-01-26 15:14 ` marxin at gcc dot gnu.org
@ 2022-01-26 15:19 ` aklitzing at gmail dot com
2022-01-27 8:38 ` marxin at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: aklitzing at gmail dot com @ 2022-01-26 15:19 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
--- Comment #5 from A. Klitzing <aklitzing at gmail dot com> ---
Created attachment 52294
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52294&action=edit
assembler output
By the way... If idx is not initialized it will work, too.
@@ -4,7 +4,7 @@
int main(int argc, char** argv)
{
const QString pVersion = QStringLiteral("1.12.2");
- int idx = 0;
+ int idx;
const auto& version = QVersionNumber::fromString(pVersion, &idx);
// #ifdef Q_CC_GNU
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (4 preceding siblings ...)
2022-01-26 15:19 ` aklitzing at gmail dot com
@ 2022-01-27 8:38 ` marxin at gcc dot gnu.org
2022-01-27 17:02 ` thiago at kde dot org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-01-27 8:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to A. Klitzing from comment #5)
> Created attachment 52294 [details]
> assembler output
>
> By the way... If idx is not initialized it will work, too.
>
> @@ -4,7 +4,7 @@
> int main(int argc, char** argv)
> {
> const QString pVersion = QStringLiteral("1.12.2");
> - int idx = 0;
> + int idx;
> const auto& version = QVersionNumber::fromString(pVersion, &idx);
>
> // #ifdef Q_CC_GNU
You were lucky in this case ;)
Anyway, upstream removed the pure attribute as we suggested:
https://codereview.qt-project.org/c/qt/qtbase/+/392357
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (5 preceding siblings ...)
2022-01-27 8:38 ` marxin at gcc dot gnu.org
@ 2022-01-27 17:02 ` thiago at kde dot org
2022-01-27 17:33 ` marxin at gcc dot gnu.org
2022-01-28 7:39 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: thiago at kde dot org @ 2022-01-27 17:02 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
--- Comment #7 from Thiago Macieira <thiago at kde dot org> ---
(In reply to Martin Liška from comment #6)
> Anyway, upstream removed the pure attribute as we suggested:
> https://codereview.qt-project.org/c/qt/qtbase/+/392357
Can we be assured the pure attribute will work for complex return types?
https://gcc.godbolt.org/z/KE4s74od3
struct S
{
bool *ptr;
S();
S(const S &);
};
#ifdef __GNUC__
__attribute__((pure))
#endif
S f1();
bool f2()
{
return *f1().ptr;
}
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (6 preceding siblings ...)
2022-01-27 17:02 ` thiago at kde dot org
@ 2022-01-27 17:33 ` marxin at gcc dot gnu.org
2022-01-28 7:39 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-01-27 17:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
@Richi: Can you chime in, please?
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug c++/104243] Optimization requires __sync_synchronize
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
` (7 preceding siblings ...)
2022-01-27 17:33 ` marxin at gcc dot gnu.org
@ 2022-01-28 7:39 ` rguenth at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-01-28 7:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104243
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Thiago Macieira from comment #7)
> (In reply to Martin Liška from comment #6)
> > Anyway, upstream removed the pure attribute as we suggested:
> > https://codereview.qt-project.org/c/qt/qtbase/+/392357
>
> Can we be assured the pure attribute will work for complex return types?
>
> https://gcc.godbolt.org/z/KE4s74od3
> struct S
> {
> bool *ptr;
> S();
> S(const S &);
> };
>
> #ifdef __GNUC__
> __attribute__((pure))
> #endif
> S f1();
> bool f2()
> {
> return *f1().ptr;
> }
I think this came up before. Yes, the intent is that this is well-defined
if 'f1' has no observable effects on the state of the program other than
to return a value. There might have been bugs in old versions of GCC
of course - I do remember some points-to analysis oddities around this,
checking GCC 7 it works OK there at least. A testcase to check would be
for example
struct S
{
bool *ptr;
S();
S(const S &);
};
__attribute__((pure))
S f1(bool *);
void f2()
{
bool c = false;
*f1(&c).ptr = true;
if (!c)
__builtin_abort ();
}
but all versions godbolt has do not miscompile this.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-28 7:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 14:37 [Bug c++/104243] New: Optimization requires __sync_synchronize aklitzing at gmail dot com
2022-01-26 15:07 ` [Bug c++/104243] " rguenth at gcc dot gnu.org
2022-01-26 15:10 ` marxin at gcc dot gnu.org
2022-01-26 15:10 ` marxin at gcc dot gnu.org
2022-01-26 15:14 ` marxin at gcc dot gnu.org
2022-01-26 15:19 ` aklitzing at gmail dot com
2022-01-27 8:38 ` marxin at gcc dot gnu.org
2022-01-27 17:02 ` thiago at kde dot org
2022-01-27 17:33 ` marxin at gcc dot gnu.org
2022-01-28 7:39 ` rguenth at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).