Page MenuHomePhabricator

tejohnson (Teresa Johnson)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 27 2015, 11:17 AM (225 w, 1 d)

Recent Activity

Yesterday

Herald added a reviewer for D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++: jdoerfert.

I had some time to work on this finally late last week. I decided the most straightforward thing was to implement the necessary interface changes to the TLI analysis to make it require a Function (without any changes yet to how that analysis operates). See D66428 that I just mailed for review. That takes care of the most widespread changes needed for this migration, and afterwards we can change the analysis to look at the function attributes and make a truly per-function TLI.

Mon, Aug 19, 10:18 AM · Restricted Project, Restricted Project
tejohnson created D66428: Change TargetLibraryInfo analysis passes to always require Function.
Mon, Aug 19, 10:15 AM · Restricted Project

Thu, Aug 15

tejohnson accepted D66253: [NewPM][PassInstrumentation] IR printing support for (Thin)LTO.

lgtm

Thu, Aug 15, 10:23 AM · Restricted Project
tejohnson added a comment to D66253: [NewPM][PassInstrumentation] IR printing support for (Thin)LTO.

Thanks for fixing this. Comment on the test case below.

Thu, Aug 15, 5:38 AM · Restricted Project

Wed, Aug 14

tejohnson created D66264: [ThinLTO] Fix handling of weak interposable symbols.
Wed, Aug 14, 5:22 PM · Restricted Project

Mon, Aug 12

tejohnson added a comment to D65726: [gold] Fix X86/strip_names.ll after r367755.

I believe this same fix was applied in r368281, and that this is now obsolete. (Sorry for the delayed review, I was out last week)

Mon, Aug 12, 7:27 AM · Restricted Project

Fri, Aug 2

tejohnson committed rG08b72f0d4161: Use llvm-nm instead of nm in new test to unbreak Windows bot (authored by tejohnson).
Use llvm-nm instead of nm in new test to unbreak Windows bot
Fri, Aug 2, 8:50 AM
tejohnson committed rL367688: Use llvm-nm instead of nm in new test to unbreak Windows bot.
Use llvm-nm instead of nm in new test to unbreak Windows bot
Fri, Aug 2, 8:49 AM
tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.

This test is failing for me:

Exit Code: 1

Command Output (stderr):
--
/home/jayfoad2/llvm-debug/bin/opt: /home/jayfoad2/git/llvm-project/llvm/test/ThinLTO/X86/Inputs/devirt3.ll: error: Could not open input file: No such file or directory

--

********************
Testing Time: 2.22s
********************
Failing Tests (1):
    LLVM :: ThinLTO/X86/devirt2.ll

  Expected Passes    : 90
  Unexpected Failures: 1

Did you forget to commit devirt3.ll? Or is it a typo for devirt2.ll?

Fri, Aug 2, 7:32 AM · Restricted Project
tejohnson accepted D65314: CodeGen: Don't follow aliases when extracting type info..

LGTM with suggestion below.

Fri, Aug 2, 7:25 AM · Restricted Project
tejohnson committed rGdeb61871d302: Fix new test try 2 (authored by tejohnson).
Fix new test try 2
Fri, Aug 2, 6:50 AM
tejohnson committed rL367682: Fix new test try 2.
Fix new test try 2
Fri, Aug 2, 6:49 AM
tejohnson committed rGe69f8dcd0b1b: Fix new test (authored by tejohnson).
Fix new test
Fri, Aug 2, 6:27 AM
tejohnson committed rL367680: Fix new test.
Fix new test
Fri, Aug 2, 6:26 AM
tejohnson committed rGd2df54e6a55a: [ThinLTO] Implement index-based WPD (authored by tejohnson).
[ThinLTO] Implement index-based WPD
Fri, Aug 2, 6:13 AM
tejohnson committed rL367679: [ThinLTO] Implement index-based WPD.
[ThinLTO] Implement index-based WPD
Fri, Aug 2, 6:13 AM
tejohnson closed D55153: [ThinLTO] Implement index-based WPD.
Fri, Aug 2, 6:13 AM · Restricted Project

Thu, Aug 1

tejohnson updated the diff for D55153: [ThinLTO] Implement index-based WPD.

Address comments

Thu, Aug 1, 2:18 PM · Restricted Project
tejohnson committed rG01dcdcdd92ef: [IR] Add getArg() method to Function class (authored by tejohnson).
[IR] Add getArg() method to Function class
Thu, Aug 1, 8:33 AM
tejohnson committed rL367576: [IR] Add getArg() method to Function class.
[IR] Add getArg() method to Function class
Thu, Aug 1, 8:32 AM
tejohnson closed D64925: [IR] Add getArg() method to Function class.
Thu, Aug 1, 8:32 AM · Restricted Project

Wed, Jul 31

tejohnson added a comment to D64925: [IR] Add getArg() method to Function class.

I can't seem to commit it myself, could someone else do it for me? Potentially @tejohnson

Wed, Jul 31, 9:13 PM · Restricted Project
tejohnson added reviewers for D65532: [LTO][Legacy] Update TargetOption after parsing debug options: mehdi_amini, probinson.
Wed, Jul 31, 2:52 PM · Restricted Project
tejohnson added a comment to D65532: [LTO][Legacy] Update TargetOption after parsing debug options.

I just discovered that there was a nearly identical patch to do this a few years ago, which generated a bunch of discussion and it seems that there was an agreement not to do this (relating to not exposing internal options through the C API, but I haven't parsed it in detail yet). See https://reviews.llvm.org/D19015

Wed, Jul 31, 2:52 PM · Restricted Project
tejohnson added a comment to D65532: [LTO][Legacy] Update TargetOption after parsing debug options.

Can you add a test case?

Wed, Jul 31, 1:41 PM · Restricted Project
tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.

Ping - @pcc any more comments?

Wed, Jul 31, 7:23 AM · Restricted Project

Tue, Jul 30

tejohnson accepted D65482: [DAGCombiner] Add an option to control whether or not to enable store merging.

lgtm

Tue, Jul 30, 3:47 PM · Restricted Project, Restricted Project

Mon, Jul 29

tejohnson accepted D64925: [IR] Add getArg() method to Function class.

lgtm

Mon, Jul 29, 8:07 AM · Restricted Project
tejohnson added a comment to D65376: Randomly outline code for cold regions.

I'm not sure it is a good idea to have non-deterministic optimizations.

Mon, Jul 29, 7:39 AM · Restricted Project
tejohnson added a comment to D64925: [IR] Add getArg() method to Function class.

Suggest sending this change with patch that uses and tests it.

@tejohnson any more recommendations?

Mon, Jul 29, 7:36 AM · Restricted Project
tejohnson accepted D65312: ThinLTOBitcodeWriter: Include globals associated with type metadata globals in the merged module..

lgtm

Mon, Jul 29, 7:36 AM · Restricted Project

Wed, Jul 24

tejohnson added a comment to D64925: [IR] Add getArg() method to Function class.

Suggest sending this change with patch that uses and tests it.

Wed, Jul 24, 12:15 PM · Restricted Project
tejohnson accepted D65115: IR: Teach GlobalIndirectSymbol::getBaseObject() to handle more kinds of expressions..

LGTM

Wed, Jul 24, 12:12 PM · Restricted Project

Tue, Jul 23

tejohnson added inline comments to D65115: IR: Teach GlobalIndirectSymbol::getBaseObject() to handle more kinds of expressions..
Tue, Jul 23, 9:25 AM · Restricted Project

Jul 19 2019

tejohnson committed rG604f802fd30d: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1 (authored by tejohnson).
[LTO] Always mark regular LTO units with EnableSplitLTOUnit=1
Jul 19 2019, 4:05 PM
tejohnson committed rL366623: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
[LTO] Always mark regular LTO units with EnableSplitLTOUnit=1
Jul 19 2019, 4:02 PM
tejohnson closed D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
Jul 19 2019, 4:02 PM · Restricted Project, Restricted Project
tejohnson retitled D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1 from [LTO] Don't mark regular LTO units with EnableSplitLTOUnit to [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
Jul 19 2019, 3:37 PM · Restricted Project, Restricted Project
tejohnson updated the diff for D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.

Address comments

Jul 19 2019, 3:37 PM · Restricted Project, Restricted Project
tejohnson abandoned D65008: [LTO] Ignore regular LTO modules when looking for partially split LTO Units.

Going with a different clang-side approach (see D65009).

Jul 19 2019, 3:34 PM · Restricted Project
tejohnson added a comment to D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
In D65009#1594009, @pcc wrote:

Sorry, just realized this. If I do

clang++ -c -flto a.cpp # "split"
clang++ -c -flto=thin b.cpp -fwhole-program-vtables # non-split
clang++ a.o b.o

this should fail, right? If I'm not mistaken, this patch series will cause this to succeed. I think we need to change this patch so that it always sets EnableSplitLTOUnit to 1 for regular LTO, then you can drop the other patch.

Jul 19 2019, 3:12 PM · Restricted Project, Restricted Project
tejohnson created D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
Jul 19 2019, 1:02 PM · Restricted Project, Restricted Project
tejohnson added a child revision for D65008: [LTO] Ignore regular LTO modules when looking for partially split LTO Units: D65009: [LTO] Always mark regular LTO units with EnableSplitLTOUnit=1.
Jul 19 2019, 1:02 PM · Restricted Project
tejohnson created D65008: [LTO] Ignore regular LTO modules when looking for partially split LTO Units.
Jul 19 2019, 1:02 PM · Restricted Project

Jul 16 2019

tejohnson added a comment to D64461: [lld-link] implement -thinlto-index-only.
In D64461#1586582, @rnk wrote:

Distributed ThinLTO doesn't support archives: http://lists.llvm.org/pipermail/llvm-dev/2019-June/133145.html. @tejohnson suggested -start-lib -end-lib as a workaround, but that's not a thing for COFF. Do you have plans for archive support, or is that not something you need to deal with for your use cases?

Personally I believe we should prioritize fixing that. It's trivial to fix for thin archives, since those are practically equivalent to the command line object groups.

Jul 16 2019, 7:06 AM · Restricted Project

Jul 15 2019

tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

AFAIU here is a coarse plan of what needs to happen

  1. Add a no-builtin clang function attribute that has the same semantic as the no-builtin cmd line argument
  2. Propagate it to the IR.
    • In the light of recent discussions and as @theraven suggested it seems more appropriate to encode them as individual IR attributes (e.g. "no-builtin-memcpy", "no-builtin-sqrt", etc..)
  3. Propagate/merge the no-builtin IR attribute for LTO by "updating AttributeFuncs::areInlineCompatible and/or AttributeFuncs::mergeAttributesForInlining and adding a new MergeRule in include/llvm/IR/Attributes.td and writing a function like adjustCallerStackProbeSize."

This one isn't about LTO, but rather the inliner. You could have different functions in the same module even without LTO that have incompatible no-builtin attributes. There isn't any propagation required for LTO.

  1. Get inspiration from TargetTransformInfo to get TargetLibraryInfo on a per function basis for all passes and respect the IR attribute.

    I'm not familiar with 3 and 4 but I can definitely have a look. I'll update this patch to do 1 and 2 in the meantime. @tejohnson let me know how you want to proceed for your related patch. I'm happy to help if I can.

I will mark that one obsolete. I can work on 4, it may just take some time to get it all plumbed.

Jul 15 2019, 10:45 AM · Restricted Project, Restricted Project

Jul 12 2019

tejohnson accepted D64610: [clang] allow -fthinlto-index= without -x ir.

LGTM with the comment fix from @MaskRay

Jul 12 2019, 5:16 PM · Restricted Project, Restricted Project

Jul 11 2019

tejohnson added a comment to D64458: add -fthinlto-index= option to clang-cl.

LGTM, but please wait for @pcc to make sure he is satisfied.
I think it would be good to port the file type deduction to clang (possibly as a follow on), to make these consistent.

Jul 11 2019, 3:32 PM · Restricted Project, Restricted Project

Jul 8 2019

tejohnson updated the diff for D55153: [ThinLTO] Implement index-based WPD.

Address comments.

Jul 8 2019, 8:29 PM · Restricted Project
tejohnson added inline comments to D55153: [ThinLTO] Implement index-based WPD.
Jul 8 2019, 8:29 PM · Restricted Project

Jul 3 2019

tejohnson accepted D64134: [scudo][standalone] Link tests against libatomic.

LGTM. Confirmed this fixes my issue. FTR I am building on a 64-bit platform.

Jul 3 2019, 8:20 AM · Restricted Project, Restricted Project
tejohnson committed rG79e50166f83c: [ThinLTO] Fix gcc warnings from commit (authored by tejohnson).
[ThinLTO] Fix gcc warnings from commit
Jul 3 2019, 8:14 AM
tejohnson committed rL365047: [ThinLTO] Fix gcc warnings from commit.
[ThinLTO] Fix gcc warnings from commit
Jul 3 2019, 8:14 AM

Jul 2 2019

tejohnson added a comment to D54155: [CodeGen] Make branch funnels pass the machine verifier.

@pcc can you take a look? The same failure just popped up in the new devirt test I added today (test added in r364960, workaround added in r364997).

Jul 2 2019, 7:19 PM
tejohnson committed rGba5a72ff8dc7: [ThinLTO] Reenable test with workaround for known failure (authored by tejohnson).
[ThinLTO] Reenable test with workaround for known failure
Jul 2 2019, 7:18 PM
tejohnson committed rL364997: [ThinLTO] Reenable test with workaround for known failure.
[ThinLTO] Reenable test with workaround for known failure
Jul 2 2019, 7:14 PM
tejohnson committed rG45fa289eb136: [ThinLTO] Work around existing failure exposed by new test (authored by tejohnson).
[ThinLTO] Work around existing failure exposed by new test
Jul 2 2019, 4:29 PM
tejohnson committed rL364978: [ThinLTO] Work around existing failure exposed by new test.
[ThinLTO] Work around existing failure exposed by new test
Jul 2 2019, 4:28 PM
tejohnson committed rG54c7907f52ea: [ThinLTO] Dump input on failure in devirt test (authored by tejohnson).
[ThinLTO] Dump input on failure in devirt test
Jul 2 2019, 3:11 PM
tejohnson committed rL364973: [ThinLTO] Dump input on failure in devirt test.
[ThinLTO] Dump input on failure in devirt test
Jul 2 2019, 3:06 PM
tejohnson committed rG5b868285ba86: [ThinLTO] Address post-review suggestions for index-based WPD summary (authored by tejohnson).
[ThinLTO] Address post-review suggestions for index-based WPD summary
Jul 2 2019, 2:11 PM
tejohnson committed rL364968: [ThinLTO] Address post-review suggestions for index-based WPD summary.
[ThinLTO] Address post-review suggestions for index-based WPD summary
Jul 2 2019, 2:11 PM
tejohnson added inline comments to D54815: [ThinLTO] Add summary entries for index-based WPD.
Jul 2 2019, 2:11 PM · Restricted Project
tejohnson committed rGf2055c5eb835: [gold] Fix test after BitStream reader error changes (authored by tejohnson).
[gold] Fix test after BitStream reader error changes
Jul 2 2019, 1:26 PM
tejohnson committed rL364965: [gold] Fix test after BitStream reader error changes.
[gold] Fix test after BitStream reader error changes
Jul 2 2019, 1:26 PM
tejohnson added inline comments to D54815: [ThinLTO] Add summary entries for index-based WPD.
Jul 2 2019, 1:20 PM · Restricted Project
tejohnson committed rGa70043632335: [ThinLTO] Add summary entries for index-based WPD (authored by tejohnson).
[ThinLTO] Add summary entries for index-based WPD
Jul 2 2019, 12:40 PM
tejohnson committed rL364960: [ThinLTO] Add summary entries for index-based WPD.
[ThinLTO] Add summary entries for index-based WPD
Jul 2 2019, 12:40 PM
tejohnson closed D54815: [ThinLTO] Add summary entries for index-based WPD.
Jul 2 2019, 12:40 PM · Restricted Project
tejohnson updated the diff for D54815: [ThinLTO] Add summary entries for index-based WPD.

Address review comment, and update test for recent auto hide summary changes

Jul 2 2019, 12:40 PM · Restricted Project
tejohnson committed rGe6768d613adb: [RA] Fix spelling of Greedy register allocator internal option (authored by tejohnson).
[RA] Fix spelling of Greedy register allocator internal option
Jul 2 2019, 11:55 AM
tejohnson committed rL364958: [RA] Fix spelling of Greedy register allocator internal option.
[RA] Fix spelling of Greedy register allocator internal option
Jul 2 2019, 11:54 AM
tejohnson accepted D63444: [ThinLTO] Optimize write-only globals out.

LGTM with one minor comment nit below. Thanks!

Jul 2 2019, 8:16 AM · Restricted Project

Jun 29 2019

tejohnson added inline comments to D63976: Allow clang -Os and -Oz to work with -flto and lld.
Jun 29 2019, 3:17 PM · Restricted Project

Jun 28 2019

tejohnson added a comment to D60495: Load balancing for LTO.

LGTM, but it'd be nice if @tejohnson or @pcc would also approve.

Jun 28 2019, 4:09 PM · Restricted Project
tejohnson added a comment to D60495: Load balancing for LTO.

Thanks! I think this makes sense, and since this is now the same as what is done in the old LTO API (ThinLTOCodeGenerator.cpp), thinking it would be better to extract into a common helper in LTO.cpp that is called from both places (that way if we find a way to improve the sorting, e.g. some of the other metrics @mehdi_amini mentioned, it only has to be changed in one place).

Jun 28 2019, 1:34 PM · Restricted Project
tejohnson added inline comments to D63444: [ThinLTO] Optimize write-only globals out.
Jun 28 2019, 12:14 PM · Restricted Project
tejohnson added a comment to D63831: [scudo][standalone] Introduce the C & C++ wrappers [fixed].

I am getting failures when trying to link these tests, due to missing symbols, e.g.:

Jun 28 2019, 11:50 AM · Restricted Project, Restricted Project

Jun 26 2019

tejohnson added a comment to D55153: [ThinLTO] Implement index-based WPD.
In D55153#1481171, @pcc wrote:

Could you please revert the changes from D54815 out of this change so that it is easier to read?

Yep, sorry, looks like that got messed up when I merged in some other changes awhile back. I'm working on merging in the changes I made this morning and last week to the underlying index patch, and then will update this to contain only the delta. Hopefully it will get updated later this afternoon.

I have rebased on top of the recent changes to D54815, and updated the patch so it reflects the diffs on top of that patch.

Jun 26 2019, 5:18 PM · Restricted Project
tejohnson added a comment to D54815: [ThinLTO] Add summary entries for index-based WPD.

ping - @pcc any more comments?

Jun 26 2019, 5:18 PM · Restricted Project

Jun 25 2019

Herald added a project to D41585: [Greedy RegAlloc] Take into account the cost of local intervals when selecting split candidate.: Restricted Project.

I was looking at flags in this file and noticed there is a typo in the flag added here (condsider-local-interval-cost should be "consider-...", note the extraneous "d" in the flag). Also, there doesn't seem to be any test utilizing this flag. Should it be removed?

Jun 25 2019, 9:39 AM · Restricted Project

Jun 21 2019

tejohnson added a comment to D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data.

Adding Wei who works on SamplePGO on our end currently.

Jun 21 2019, 6:54 AM · Restricted Project, Restricted Project
tejohnson added a reviewer for D63626: [clang][NewPM] Remove exception handling before loading pgo sample profile data: wmi.
Jun 21 2019, 6:49 AM · Restricted Project, Restricted Project

Jun 20 2019

tejohnson added a comment to D63444: [ThinLTO] Optimize write-only globals out.

Thanks! A bunch of misc comments/suggestions below.

Jun 20 2019, 2:00 PM · Restricted Project

Jun 19 2019

tejohnson added a comment to D63444: [ThinLTO] Optimize write-only globals out.

I didn't get very far today, will finish the review tomorrow morning.

Jun 19 2019, 10:09 PM · Restricted Project

Jun 14 2019

tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Jun 14 2019, 9:54 AM · Restricted Project, Restricted Project
tejohnson added a comment to D59673: [Clang] Harmonize Split DWARF options with llc.

lgtm for the LTO bits. Suggestion below for comment.

Jun 14 2019, 7:09 AM · Restricted Project, Restricted Project

Jun 12 2019

tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Jun 12 2019, 12:54 PM · Restricted Project, Restricted Project
tejohnson accepted D63078: [ThinLTO][Bitcode] Add 'entrycount' to FS_COMBINED_PROFILE. NFC.

Interesting - does this mean we just encoded it with a VBR 4 (what numrefs was to be encoded with), and everything else just got shifted down in the encoding (so immutablerefcnt got encoded as the first VBR 8 in the array at the end)? Unfortunate that there isn't a way to test for this.

Jun 12 2019, 12:06 PM · Restricted Project
tejohnson added inline comments to D59673: [Clang] Harmonize Split DWARF options with llc.
Jun 12 2019, 6:39 AM · Restricted Project, Restricted Project

May 29 2019

tejohnson committed rG5b2088d1fac1: [ThinLTO] Use original alias visibility when importing (authored by tejohnson).
[ThinLTO] Use original alias visibility when importing
May 29 2019, 9:50 AM
tejohnson committed rL361989: [ThinLTO] Use original alias visibility when importing.
[ThinLTO] Use original alias visibility when importing
May 29 2019, 9:50 AM
tejohnson closed D62535: [ThinLTO] Use original alias visibility when importing.
May 29 2019, 9:50 AM · Restricted Project

May 28 2019

tejohnson created D62535: [ThinLTO] Use original alias visibility when importing.
May 28 2019, 11:04 AM · Restricted Project

May 23 2019

tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

AFAIU here is a coarse plan of what needs to happen

  1. Add a no-builtin clang function attribute that has the same semantic as the no-builtin cmd line argument
  2. Propagate it to the IR.
    • In the light of recent discussions and as @theraven suggested it seems more appropriate to encode them as individual IR attributes (e.g. "no-builtin-memcpy", "no-builtin-sqrt", etc..)
  3. Propagate/merge the no-builtin IR attribute for LTO by "updating AttributeFuncs::areInlineCompatible and/or AttributeFuncs::mergeAttributesForInlining and adding a new MergeRule in include/llvm/IR/Attributes.td and writing a function like adjustCallerStackProbeSize."
May 23 2019, 10:00 PM · Restricted Project, Restricted Project
tejohnson abandoned D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

As per D61634, we are going to use function attributes instead.

May 23 2019, 10:00 PM · Restricted Project, Restricted Project

May 17 2019

tejohnson added a comment to rL309966: Disable loop peeling during full unrolling pass..

I may not understand the implementation completely. So could you explain a little bit how this patch only affect peeling for full unrolling? It seems to me you disable peeling for all the unrolling (full, partial or run-time). i.e, where is the code that you distinguish peeling according to unrolling type? I cannot find that logic. Thanks a lot!

May 17 2019, 10:53 AM

May 15 2019

tejohnson added a comment to D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends.

There is discussion of using function attributes to control this instead, see https://reviews.llvm.org/D61634.
I'll hold off on making the planned changes here until the overall approach is decided.

May 15 2019, 8:02 AM · Restricted Project, Restricted Project
tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

Using function level attributes instead of module flags does provide finer grained control and avoids the conservativeness when merging IR for LTO. The downsides I see, mostly just in terms of the engineering effort to get this to work, are:

  • need to prevent inlining with different attributes

IIUC this is needed regardless of the proposed change. Correct?

May 15 2019, 8:00 AM · Restricted Project, Restricted Project

May 14 2019

tejohnson added a comment to D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++.

I have a related patch that turns -fno-builtin* options into module flags

Do you have any opinion on representing -fno-builtin using a module flag vs. a function attribute in IR? It seems generally more flexible and easier to reason about a function attribute from my perspective. But I might be missing something about the semantics of -fno-builtin that would make that representation awkward. Or I guess it might just be more work to implement, given we have some IPO passes that use TargetLibraryInfo?

I think that a function attribute would be better. We generally use these flags only in the context of certain translation units, and when we use LTO, it would be sad if we had to take the most-conservative settings across the entire application. When we insert new function call to a standard library, we always do it in the context of some function. We'd probably need to block inlining in some cases, but that's better than a global conservative setting.

May 14 2019, 3:53 PM · Restricted Project, Restricted Project