[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-27 Thread John Paul Adrian Glaubitz via Phabricator via cfe-commits
glaubitz added a comment. Not sure if this is related, but on SPARC, stage2 builds recently started to fail with: [379/5552] Building CXX object projects/compiler-rt/lib/ubsan/CMakeFiles/RTUbsan.sparc.dir/ubsan_diag.cpp.o FAILED:

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-25 Thread Venkataramanan Kumar via Phabricator via cfe-commits
venkataramanan.kumar.llvm added a comment. In D88712#2414171 , @spatel wrote: > In D88712#2413877 , > @venkataramanan.kumar.llvm wrote: > >> In D88712#2413688 , @spatel

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88712#2413877 , @venkataramanan.kumar.llvm wrote: > In D88712#2413688 , @spatel wrote: > >> In D88712#2412874 , >> @venkataramanan.kumar.llvm

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Venkataramanan Kumar via Phabricator via cfe-commits
venkataramanan.kumar.llvm added a comment. In D88712#2413688 , @spatel wrote: > In D88712#2412874 , > @venkataramanan.kumar.llvm wrote: > >> In D88712#2412366 , @MaskRay

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-24 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D88712#2412874 , @venkataramanan.kumar.llvm wrote: > In D88712#2412366 , @MaskRay wrote: > >> In D88712#2411841 , >> @venkataramanan.kumar.llvm

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D88712#2412874 , @venkataramanan.kumar.llvm wrote: > In D88712#2412366 , @MaskRay wrote: > >> In D88712#2411841 , >> @venkataramanan.kumar.llvm

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-23 Thread Venkataramanan Kumar via Phabricator via cfe-commits
venkataramanan.kumar.llvm added a comment. In D88712#2412366 , @MaskRay wrote: > In D88712#2411841 , > @venkataramanan.kumar.llvm wrote: > >> > > For your example: > > extern double log (double) asm (""

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D88712#2411841 , @venkataramanan.kumar.llvm wrote: > For your example: extern double log (double) asm ("" "log_finite") __attribute__ ((nothrow)); double mylog (double d) { return log(d); } The intention is to emit

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-11-23 Thread Venkataramanan Kumar via Phabricator via cfe-commits
venkataramanan.kumar.llvm added a comment. Hi, I am on Ubuntu 18 machine and it has finite math header . This header is included by the glibc 2.27. This header has this following definition. extern double log (double) __asm__ ("" "__log_finite") __attribute__ ((__nothrow__ )); Consider the

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Fangrui Song via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5a338599fbaa: [CGBuiltin] Respect asm labels and redefine_extname for builtins with… (authored by MaskRay). Repository: rG LLVM Github Monorepo

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 298480. MaskRay edited the summary of this revision. MaskRay added a comment. Add an example about inconsistency. Elaborate on the limitation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88712/new/

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D88712#2333294 , @rsmith wrote: > If the answer is that we consider (2) to be incorrect, but that this is only > a partial step in that direction, I think that's fine -- that's enough that > we can know where the bug is if /

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. If the answer is that we consider (2) to be incorrect, but that this is only a partial step in that direction, I think that's fine -- that's enough that we can know where the bug is if / when

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a subscriber: zatrazz. MaskRay added a comment. In D88712#2333030 , @rsmith wrote: > I worry that we're chasing after the implementation details of GCC here. It > seems self-contradictory to say all three of these things at once: > > 1. The

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. Disabling builtin handling when `asm` is enabled would be a major annoyance for NetBSD. The interaction between symbol renaming and TLI is complicated. There are cases where it would make perfect sense and others were it would be harmful. Example of the latter is the way

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-15 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. I worry that we're chasing after the implementation details of GCC here. It seems self-contradictory to say all three of these things at once: 1. The frontend function `foo` has known, builtin semantics X. (Eg, we'll use those semantics in the frontend.) 2. The symbol

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In case my previous comment is not clear: we can do renaming in LLVM, but the benefit is small (for a few libcalls (only some really simple libcalls) with custom code emitting, if they have `asm(...)`, they are now optimizable). We will require a renaming

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D88712#2326223 , @rsmith wrote: > In D88712#2325823 , @MaskRay wrote: > >> In D88712#2324105 , @rsmith wrote: >> >>> What are the expected

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-12 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D88712#2325823 , @MaskRay wrote: > In D88712#2324105 , @rsmith wrote: > >> What are the expected semantics in this case? Is this: >> >> - the function is still the builtin, but if it ends

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D88712#2324105 , @rsmith wrote: > What are the expected semantics in this case? Is this: > > - the function is still the builtin, but if it ends up being a libcall, call > a function with a different asm name, or > - the

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-12 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 297667. MaskRay edited the summary of this revision. MaskRay added a comment. Mention the limitation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88712/new/ https://reviews.llvm.org/D88712 Files:

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-11 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. What are the expected semantics in this case? Is this: - the function is still the builtin, but if it ends up being a libcall, call a function with a different asm name, or - the function is not considered to be the builtin any more, or - something else? I think this

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Ping :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88712/new/ https://reviews.llvm.org/D88712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay updated this revision to Diff 295696. MaskRay added a comment. Improve tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88712/new/ https://reviews.llvm.org/D88712 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

2020-10-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay created this revision. MaskRay added reviewers: joerg, jyknight, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. MaskRay requested review of this revision. rL131311 added `asm()` support for builtin functions, but `asm()`