This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] snprintf optimizations
ClosedPublic

Authored by xbolva00 on Apr 30 2018, 1:32 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

xbolva00 created this revision.Apr 30 2018, 1:32 PM
xbolva00 updated this revision to Diff 144627.Apr 30 2018, 1:34 PM
xbolva00 updated this revision to Diff 144779.May 1 2018, 2:03 PM

Small formatting fix.

friendly ping

rja added a subscriber: rja.May 8 2018, 3:45 PM
rja added inline comments.
test/Transforms/InstCombine/snprintf.ll
10 ↗(On Diff #144779)

Remove dso_local

Add test for

snprintf(buf, 0, fmt)

xbolva00 updated this revision to Diff 145822.May 8 2018, 5:23 PM
xbolva00 marked an inline comment as done.
rja accepted this revision.EditedMay 8 2018, 5:33 PM

LG.

This revision is now accepted and ready to land.May 8 2018, 5:33 PM
This revision was automatically updated to reflect the committed changes.
xbolva00 updated this revision to Diff 145908.May 9 2018, 6:41 AM

Fixed operand

xbolva00 reopened this revision.May 9 2018, 7:00 AM
This revision is now accepted and ready to land.May 9 2018, 7:00 AM
This revision was automatically updated to reflect the committed changes.

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

xbolva00 added a comment.EditedMay 10 2018, 12:43 PM

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

C standard says:
int snprintf(char * restrict s, size_t n, const char * restrict format, ...);

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

C standard says:
int snprintf(char * restrict s, size_t n, const char * restrict format, ...);

Yes, but that's not an argument for making the compiler trigger a failed assertion if faced with code that doesn't exactly comply with it. Reverted until this issue is fixed.

xbolva00 added a comment.EditedMay 10 2018, 2:30 PM

This caused lots of assertion errors in building code with mingw-w64 headers (where snprintf can be a static inline function), see https://bugs.llvm.org/show_bug.cgi?id=37408. I would like to revert this change to unbreak things until this can be fixed.

C standard says:
int snprintf(char * restrict s, size_t n, const char * restrict format, ...);

Yes, but that's not an argument for making the compiler trigger a failed assertion if faced with code that doesn't exactly comply with it. Reverted until this issue is fixed.

But I am not sure how to fix. This is not related to just snprintf case at all then.

@efriedma Eli maybe knows better what we should do, so please Eli can you look at that crash?

Pinging @spatel if he can help us with a fix.

xbolva00 reopened this revision.May 11 2018, 1:33 AM
This revision is now accepted and ready to land.May 11 2018, 1:33 AM
xbolva00 updated this revision to Diff 146360.May 11 2018, 10:53 AM
This revision was automatically updated to reflect the committed changes.