[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-10-30 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein added reviewers: MaskRay, aaron.ballman. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Signed-off-by: Itay Bookstein Repository: rG LLVM Github Monorepo https://reviews.llvm.or

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-31 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Could you commit this on my behalf? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112349/new/ https://reviews.llvm.org/D112349 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-10-31 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 383661. ibookstein added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.org/D112868 Files: clang/lib/CodeGen/CodeGenModule.cpp clang/test/Sema/att

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-02 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Hmm. When I try to compile an object file where the resolver is a declaration, both clang-13, clang-14, and gcc-9.3 complain that the ifunc must point to a defined function: void *foo_resolver(); void foo(void) __attribute__((ifunc("foo_resolver"))); clang (13 a

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-03 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Note that (as the examples demonstrate) clang self-verifies and checks among other things that ifuncs that it emits point to definitions; this happens in `CodeGenModule::checkAliases()`. I haven't read the cpu_specific/cpu_dispatch-related code in CodeGenModule yet,

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-03 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. I'll first explain my thought process about the representation of aliases and ifuncs in the IR, and why I think both aliasees and resolvers must always be defined; I hope I'm not completely off track and would love it if @MaskRay could weigh in as to whether I make s

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-04 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. I'm referring you again to the start of my explanation (the first three paragraphs); the object file format (ELF) literally cannot express the semantics you're asking for. You're asking for it to support a symbol in a special kind of undefined state:

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-04 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. It sort-of-works only because you define the ifunc in both translation units (with the same name). But looks like it behaves incorrectly for references to the ifunc in the translation unit where the resolver is only declared, not defined: > cat exa

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-04 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. > I don't know much about the ELF format... but this works today? We can > define a resolver in a different TU and it WORKS thanks to the linker? So > there is perhaps something? The ifunc symbol that is emitted in the TU with the undefined resolver

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-04 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. That feature already exists - use a plain old function declaration :) My mental model for this is like this: memcpy one of the is the most widely popular APIs commonly implemented as an ifunc. In clients of this API, it's just a plain old function decl

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-04 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. I see. What is the guiding principle there, though? Generating correct IR "up front" / "the first time" rather than "fixing it up as you go via manipulations"? (could you give a link?) I can see the engineering consideration in not letting IR manipulations creep into

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-11-05 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:320 // linear structure. static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) { + const llvm::Constant *C; erichkeane wrote: > Can you explain a bit bett

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-05 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. And how is Cling expecting CFE to deal with partial knowledge situations at the implementation level? To deal with exactly the non-local cases that the current violations address? If there's no prescriptive way forward to dealing with these cases (so they're tech deb

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-05 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. I feel like we're getting lost in the weeds here. At the time a bitcode module is finalized, it is supposed to be in a valid state. The LLVM bitcode verifier does not consider GlobalAliases which have either a null or an undefined aliasee to be valid. The same shoul

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-11-05 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385211. ibookstein added a comment. Changed to using D->hasAttr() and passing that bool into the checkAliasedGlobal function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.or

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-11-05 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385212. ibookstein added a comment. Nicer param order for checkAliasedGlobal.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.org/D112868 Files: clang/lib/CodeGen/CodeGenMo

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-06 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein added reviewers: dblaikie, rjmccall. Herald added a subscriber: ormris. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Previously, the Backend_Emit{Nothing,BC,LL} modes did not run th

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-06 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385298. ibookstein added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113352/new/ https://reviews.llvm.org/D113352 Files: clang/lib/CodeGen/BackendUtil.cpp Index: clang/lib/Code

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-07 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385361. ibookstein edited the summary of this revision. ibookstein added a comment. Herald added subscribers: steven_wu, hiraditya. - Fixed PR comment about documentation - Amended clang/test/CodeGen/lto-newpm-pipeline.c to reflect change - Fixed small bug

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-07 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein marked an inline comment as done. ibookstein added a comment. In D113352#3114009 , @dblaikie wrote: > I guess CodeGenOpts::VerifyModule is on by default, but I'll admit that > surprises me - for API-built modules, I figured it was expected the

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-07 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5016 llvm::Constant *Resolver = - GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD, + GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-07 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Thanks; Might I ask that you commit this on my behalf? :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113352/new/ https://reviews.llvm.org/D113352 ___ cfe-commits mailing li

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385541. ibookstein added a comment. clang-format + fastforward rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.org/D112868 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. > To me the 'not in the weeds' part is, "How do I get CPU-dispatch/CPU-specific > to work without RAUW, since that is offensive to the CFE code owner? > Additionally, I found that some of the examples without the defined resolver > actually DO work in my downstream,

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385603. ibookstein retitled this revision from "[Sema] Diagnose and reject non-function ifunc resolvers" to "[CodeGen] Diagnose and reject non-function ifunc resolvers". ibookstein added a comment. move attr-ifunc.c test to codegen Repository: rG LLVM

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385606. ibookstein added a comment. move test from Sema to CodeGen (trying to make arcanist send multiple separate commits..?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.o

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein published this revision for review. Herald added a project: clang. Herald added a subscriber: cfe-commits. Signed-off-by: Itay Bookstein Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D113431 Files: clang/test/CodeGen/attr-ifunc.c

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-11-08 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Who is the Clang CFE maintainer that we need to involve? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112349/new/ https://reviews.llvm.org/D112349 ___ cfe-commits mailing lis

[PATCH] D113504: [clang][test][NFC] clang-format attr-ifunc.c test

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein added reviewers: erichkeane, aaron.ballman. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Signed-off-by: Itay Bookstein Repository: rG LLVM Github Monorepo https://reviews.llvm

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385923. ibookstein added a comment. add parent clang-format commit to pass CI Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113431/new/ https://reviews.llvm.org/D113431 Files: clang/test/CodeGen/attr-ifun

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 385926. ibookstein added a comment. rebase fix after adding parent NFC commits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/ https://reviews.llvm.org/D112868 Files: clang/lib/CodeGen/CodeGenMo

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. If you meant fusing the clang-format commit with this one, doing it in the same commit results in git no longer detecting the connection between them (similarity too low), so it loses the history. When searching I found a recommendation

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Yeah, it's real ugly from a CFE/LLVM separation POV. I also can't avoid seeing it as a duplication of the verifier's logic. But emitting diagnoses is better than asserting/crashing... For the recursion, maybe an equivalent of `getAliaseeObject` on `GlobalDecl`-s coul

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Right; well, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113431/new/ https://reviews.llvm.org/D113431 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:/

[PATCH] D113504: [clang][test][NFC] clang-format attr-ifunc.c test

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4f04f7d816f3: [clang][test][NFC] clang-format attr-ifunc.c test (authored by ibookstein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113504/new/ https://

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGce91540beeff: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen (authored by ibookstein). Changed prior to commit: https://reviews.llvm.org/D113431?vs=385923&id=385966#toc Repository: rG

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3b1fd19357be: [CodeGen] Diagnose and reject non-function ifunc resolvers (authored by ibookstein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112868/new/

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

2021-11-09 Thread Itay Bookstein via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9efce0baee4b: [clang] Run LLVM Verifier in modes without CodeGen too (authored by ibookstein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113352/new/ htt

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2022-12-28 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein created this revision. nextsilicon-itay-bookstein added a reviewer: jdoerfert. Herald added subscribers: guansong, yaxunl. Herald added a project: All. nextsilicon-itay-bookstein requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald ad

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-21 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 507121. nextsilicon-itay-bookstein added a comment. Herald added subscribers: mattd, asavonic, zzheng, kbarton, nemanjai. Update relevant test outputs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-21 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. I had to cut context lines to 2 to fit in the 8MB limit. It looks like there are a few files that absorb way more line diff than makes sense, but I haven't investigated way. One particular offender is `clang/test/OpenMP/nvptx_SPMD_codegen.cpp`, which

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-22 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 507532. nextsilicon-itay-bookstein added a comment. Herald added subscribers: kosarev, kerbowa, jvesely. Removed the empty string in the `getName()` invocation for the outlined helper's name. CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-24 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 508054. nextsilicon-itay-bookstein added a comment. Minor fix to the clang/CodeGen/ppc64le-varargs-f128.c test. @jdoerfert Does the PR CI run these, or are there build bots that cover the different target-offloading variants? Can I somehow

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-24 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 508059. nextsilicon-itay-bookstein added a comment. Retry, wrong patch uploaded last time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-24 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 508116. nextsilicon-itay-bookstein added a comment. Another minor tweak to clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 Files: cla

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-03-28 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. Passes CI. How should I proceed w.r.t. libomptarget tests? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-01 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. I don't have non-cumbersome access to GPU targets at the moment. I have run the following locally with the patch, and it seems to pass: > ninja check-libomptarget [0/1] Running libomptarget tests Testing Time: 9.69s Unsupported: 57 Pas

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-05 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. Ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-17 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. Ping (: CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-18 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein added a comment. Alright, cool, thanks! I'll land it tomorrow evening if there are no objections. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722 ___ cfe-commits mailing li

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-19 Thread Itay Bookstein via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG029bfc311d4d: [OpenMP] Prefix outlined and reduction func names with original func's name (authored by nextsilicon-itay-bookstein). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-19 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein reopened this revision. nextsilicon-itay-bookstein marked 2 inline comments as done. nextsilicon-itay-bookstein added a comment. This revision is now accepted and ready to land. Reopening due to revert, will update with a fixed patch Repository: rG LLVM Github Monore

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-19 Thread Itay Bookstein via Phabricator via cfe-commits
nextsilicon-itay-bookstein updated this revision to Diff 515025. nextsilicon-itay-bookstein edited the summary of this revision. nextsilicon-itay-bookstein added a comment. Updated a couple of tests that were introduced in the interim Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACT

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

2023-04-19 Thread Itay Bookstein 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 rG782c59a4eef0: [OpenMP] Prefix outlined and reduction func names with original func's name (authored by nextsilicon-itay-bookstein). Repository: rG

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

2022-01-31 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein added a reviewer: erichkeane. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Signed-off-by: Itay Bookstein Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D118619

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

2022-01-31 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Thanks! CI failures seem to be in completely unrelated libarcher, which are shared by other runs in the past few hours; landing this, then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118619/new/ https://reviews.llvm.

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

2022-01-31 Thread Itay Bookstein 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 rG2a868802a372: [clang][CodeGen][NFC] Remove unused CodeGenModule fields (authored by ibookstein). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2022-02-19 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. I now realize that the type check isn't correct for the platforms which pass arguments to the resolver. Unfortunate that the glibc wiki doesn't mention this (as far as I can tell)... I thought that the bitcast-to-"expected"-type should shield from that error, but may

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-21 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein added reviewers: erichkeane, rsmith, MaskRay. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The purpose of this change is to fix the following codegen bug: // main.c __attribute__((

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-21 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 410360. ibookstein added a comment. clang-format + description wording Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120266/new/ https://reviews.llvm.org/D120266 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Yeah, that's what happens with this patch; Reference binds against an `llvm::Function` declaration, linker resolves it to the actual ifunc in another translation unit and therefore emits IFUNC relocation. Thinking about it more, this is inelegant. I would have liked

[PATCH] D120567: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-25 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein created this revision. ibookstein requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The purpose of this change is to fix the following codegen bug: // main.c __attribute__((cpu_specific(generic))) int *foo(void) { static int

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-25 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 411409. ibookstein edited the summary of this revision. ibookstein added a comment. Changed code to generating a function declaration and 'upgrading' it to an ifunc instead of generating and ifunc and 'downgrading' it to a function declaration. I decided ag

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-25 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Ah, I saw your comment just now, good thing I didn't continue down that plain-alias-name route then! The change now satisfies that requirement in a way that binding against the alias name indeed would not: TU1 will have the callsite in `caller` bind against `foo.ifun

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-25 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 411521. ibookstein added a comment. Add tests for specific>usage>dispatch and dispatch>usage>specific orderings. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120266/new/ https://reviews.llvm.org/D120266 Fi

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

2022-02-26 Thread Itay Bookstein 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 rGf3480390be61: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers (authored by ibookstein). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 381735. ibookstein set the repository for this revision to rG LLVM Github Monorepo. ibookstein added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed type verification to look at the resolver operand rather than the

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 381751. ibookstein added a comment. rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112349/new/ https://reviews.llvm.org/D112349 Files: clang/lib/CodeGen/CodeGenModule.cpp llvm/lib/IR/Globals.cpp llvm/lib/IR/Verifier.cpp llvm/test/A

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 381754. ibookstein added a comment. Now using arcanist because commit includes change to binary file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112349/new/ https://reviews.llvm.org/D112349 Files: clan

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein added a comment. Because this commit changes an existing binary bitcode file, and because that file specifically tests backwards compatibility, does that mean I need to avoid changing it and instead add a backwards compatibility fix to the BitcodeReader? (Something like always bitcas

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-23 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 381760. ibookstein edited the summary of this revision. ibookstein added a comment. Herald added a subscriber: nemanjai. Added check to BitcodeReader, fixed clang tests, hoisted logic to shared static function on GlobalIFunc. Repository: rG LLVM Github

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

2021-10-24 Thread Itay Bookstein via Phabricator via cfe-commits
ibookstein updated this revision to Diff 381773. ibookstein added a comment. Change BitcodeReader to transparently fix up the type rather than returning an error because older formats use wrong type. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11