- User Since
- Nov 30 2017, 10:52 AM (185 w, 6 d)
Thu, Jun 10
May 3 2021
LGTM once you've addressed @phosek's nits.
Mar 31 2021
Mar 30 2021
Mar 25 2021
I will take another look over this commit in the morning. Overall looks good.
Mar 3 2021
This makes sense to me. I approve. Can we move the 3/60 seconds number to a const int value set somewhere higher up in the file as a global with a comment explaining this as well?
Feb 16 2021
If @compnerd has no gripes go ahead and land. LGTM.
Before landing, please make sure to run the check-clang tests as well because the interface stubs generation on the clang driver side invokes llvm-ifs.
Feb 1 2021
Ping? Haven't seen any chatter in a while. Anything new here? I don't want to hold you folks up.
Jan 24 2021
@wolfgangp I get this warning on macOS. I noticed it wasn't fixed in https://github.com/llvm/llvm-project/commit/c6e8f81410a2942b5abd112aa6e468268e01d946#diff-67f7be4aff998787c9c4efcb95e5d0dc225e1dcd37b9b005a3db5493f02efdc3 so I posted this small diff.
Jan 20 2021
Is this still open? Seems good to go.
Jan 12 2021
Jan 11 2021
- I think for unification, the first thing would be unifying the text abi file format. Currently, the YAML format used by ifs is very similar to the one used in InterfaceStub(elfabi), however there are not compatible. One thing is that ifs embeds llvm triples in the text ifs file while elfabi does not do that. Elfabi get platform and endianness information from command line instead of getting them from the text stub file. We did that because there are use cases that we generate ELF stubs for multiple platforms using a single set of text abi files. Is there a specific reason why ifs text files require llvm triples? Is it feasible to make llvm-ifs to get the platform information from the command line options?
Oct 27 2020
Updated based on @paquette's feedback. This only includes the StableHashTree data structure and a unit test.
Oct 23 2020
Nice. Will you folks be updating llvm-ifs to use this support instead of yaml2obj soon? If so add me to the review, I will be happy to take a look.
Oct 20 2020
Thanks for fixing this @cishida!
I think I am overall ok with this for now until the TBE folks post some more patches to integrate ifs and tbe.
Oct 7 2020
@vsk any update on this? Is there anything we can do to help in landing this patch in llvm-project/main?
Oct 1 2020
Overall looks good.
Sep 24 2020
spelling and grammar
Cleaning up patch to be easier to understand.
@sdesmalen @efriedma @dmgreen Hi Sander, Eli, Dave. Would any of you have some time to help review this Prolog Epilog Size optimization patch? Much appreciated if you do. Me and @kyulee would be available to chat on IRC or discord about it for more info.
Sep 23 2020
Sep 18 2020
Sep 15 2020
Sep 14 2020
Sep 8 2020
Sep 3 2020
Sep 2 2020
I've updated the diff to reflect the alternate non-wrapper exposure. This way requires the sanitizing of the exported objc_direct method name.
Sep 1 2020
Aug 29 2020
Aug 26 2020
Aug 25 2020
Aug 23 2020
LGTM. Go ahead and land it.
Aug 20 2020
@lanza I did it as a CodeGen option for now because we don't want anything like this to be the default until the ABI is fleshed out.
I think one danger in altering the name of the function to some extent is you dont want to clash potentially with user defined C functions.
Aug 19 2020
Ok, LGTM. I will land for you, but in your name on git. Thanks for this patch @kyulee.
Aug 18 2020
Aug 17 2020
change for clang-tidy and clang-format.
I think this LGTM. @kyulee are there any other tests that use this flag? Can you add a test that both adds and also removes some different attributes. Also a test that adds and removes he same attribute too would be good.
Good stuff. Thanks for replying so quickly @fhahn :-)
Aug 15 2020
@fhahn I ran into an llvm_unreachable when building matrix-types-spec.cpp as part of an llvm-test-suite build using -Oz with -flto=thin. I reduced the IR that hit the unreachable to:
Aug 14 2020
Please Update Diff.
Aug 11 2020
Aug 10 2020
Overall this change makes sense. I don't see why we shouldn't have this ability to remove attrs.
Closing, cause phab doesn't seem to be auto-closing.
Landed at https://github.com/llvm/llvm-project/commit/7bc03f55539f7f081daea5363f2e4845b2e75f57