Comments for the test updated. Thanks for the comment!

Friendly ping. Thanks!

@rsmith I see. Thank you for the clarification!

I think the context is Derived here. My understanding of http://wg21.link/p0968r0#2227 (in this patch's context) is that when Derived is aggregate initialized, the destructor for each element of Base is potentially invoked as well.

@rsmith @dblaikie Thank you for the comments! It seems that this is not the appropriate way to handle the issue. I'll find different way to resolve the problem.

@dblaikie I see. The problem we're experiencing is that with Clang's naming scheme we end up having different function name between the original source and the preprocessed source (as macro expansion changes the column number). This doesn't work well for me if I want to use distcc on top of our caching system, as the caching scheme expects the output to be same as far as the original source has not been changed (regardless of it's compiled directly or first preprocessed then compiled), but the distcc preprocesses the source locally then sends it to the remote build machine (and we do not turn distcc on for all workflow). I wonder if you have any suggestion to resolve this issue? Thanks!

@aprantl It is a debug info. If you compile test/CodeGenCXX/debug-info-anonymous.cpp file with clang -g2 -emit-llvm -S , you will find debug metadata something like distinct !DISubprogram(name: "foo<X::(anonymous enum at /home/twoh/llvms/llvm/tools/clang/test/CodeGenCXX/debug-info-anonymous.cpp:9:3)>", linkageName: "_Z3fooIN1XUt_EEiT_" ..., which will eventually be included in .debug_info section.

Remove conflict line.

@joerg Sorry but I'm not sure if I understand your question. This doesn't pretend to honor source code order, but makes linker to place "hot" functions under .text.hot section (There's no guarantee of ordering between functions inside .hot.text section) while "cold" functions under .text.unlikely section. This is purely for performance.

Rebase. Tests are provided in the clang counterpart (D34796).

Rebase. Sorry I somehow missed the recent comments. I addresses @davidxl's comment on documentation. Thanks!

ping!

Addressing comments from @echristo. Reverted option name change, and added a test case. Sorry I haven't work on this code for a while so it took time to invent a test case.

Hello, I observed a case where atomic builtin generates libcall when the corresponding sync builtin generates an atomic instruction (https://bugs.llvm.org/show_bug.cgi?id=38846). It seems that the alignment checking for __atomic builtins (line 759 of this patch) results the difference, and wonder if the check is actually necessary. Could anyone please shed some light on understanding this? Thanks!

Hello, I wonder if we need to keep linkonce_odr symbols live here as well. I observe a case that a vtable for template class initiated has linkonce_odr linkage and marked dead here, which results compiler crash at WholeProgramDevirt because the global variable for vtable doesn't have initializer (https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/IPO/WholeProgramDevirt.cpp#L676 assumes that GV has the initializer). Thanks!

Addressing comments from @Hahnfeld. Thanks!

Addressing comment from @mssimpso. Thanks!

Hello @jlpeyton @tlwilmar, is there a chance that this breaks runtime/test/ompt/misc/api_calls_from_other_thread.cpp test? ompt_get_num_places() is expected to return 0 from the test but with this patch it returns 1, and I think it is because kmp_affinity_num_masks is set to 1 from kmp_create_affinity_none_places().

Addressing comments from @fhahn and @junbuml's comments. Thank you!

Thanks @fhahn for the comments! I added comments about why we don't perform splitting for the callsites inside the landing pad. Please let me know if you think it is too verbose.

Addressing @junbuml's comments. Thanks!

Update test to check f2.llvm.0 as well.

@tejohnson Thanks for the clarification. Regarding hotness, I'm not sure if providing "some" hotness is better than leaving it as unknown if profile data is not provided (If profile data is given, as you said, VP metadata will be attached to the callsite). I'm afraid that synthesized hotness may confuse optimizers, but please let me know if you have different idea.

@tejohnson I think your right. What I meant was that when the metadata is imported to bar.o, it references f1 and f2 by their promoted names, which makes the declarations with the promoted names to be added. Did I get it right, or still miss something?

clang-format

@jkorous-apple Got it. I agree that it would be better to move the comments to the header. Will land it soon. Thanks!

Thanks @jkorous-apple for the comment. I think your suggestion is a more
precise description for the implementation, and adjusted the comments
accordingly.

@jkorous-apple Thanks for the comments! Yeah, I was thinking of O(lenght_of_string) approach, but considering the complicatedness of the implementation (I guess the real implementation would be a bit more complex than your pseudo implementation to handle quote and '\n\r' '\r\n' cases) I decided to stay with O(length_of_string * number_of_endlines_in_string) but optimizing the number of move operations.

Thanks @jkorous-apple for your comments. I modified the type for the variables and replaced unnecessary inserts and erases with updates.

Addressing @vsapsai's comments. Thank you for the suggestion! Added test case actually finds an off-by-one error in the original patch. I improved the comments as well.

Ping. Thanks!

Ping. Thanks!

clang-format

Ping. Thanks!

Ping, Thanks!

Addressing @davidxl and @xur's comment. Thanks!

Fix a test.

Friendly ping. @davidxl, I think there's no harm to make clang consistent with gcc for compiler options, and I wonder if you have any concerns that I may miss. Thanks!

Ping. Thanks!

Addressing dblaikie's comments. Thanks!

Relocate.

Ping. Thanks!

I think it is generally good to match what GCC does to not to confuse people.

Update documentation. Please let me know if I need to update other documents as well. Thanks!

@davidxl I think it is theoretically possible, if the if branch is not taken on line 294. Did I miss something? Thanks!

Ping. Thanks!

Ping. Thanks!

Delete unnecessary line from the test.

@wmi Good call! I fixed the code per your suggestion. Thanks!

minor fix.

Addressing comments from @wmi. Thank you for the suggestion!

https://reviews.llvm.org/D34796 is clang side change.

Ping. Thanks!

Hello @Ayal, can you please update the comments per @hfinkel's request so that I can accept the patch? Thanks!

Addressing @wmi's concern by limiting the targets to the recurrence cycles that only the last instruction of the recurrence (that feeds the PHI instruction) can have uses outside of the recurrence. This is not an ideal solution yet, and more fundamental solution (such as having recurrence optimization as a separate pass and/or using live range analysis for it) should follow. But still I think it is worth to have it here.

In D34373#785642, @hfinkel wrote:

In D34373#785485, @twoh wrote:

In D34373#785367, @hfinkel wrote:

In D34373#784975, @twoh wrote:

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!

I know that we're currently missing opportunities for large vectorizable loops with low (static) trip counts. Smaller inner loops are also good candidates for unpredicated vectorization, but we may need to be a bit careful because of modeling inaccuracies and phase-ordering effects (e.g. if we don't vectorize a loop, then we'll end up unrolling it when the unroller runs).

Got it. My concern was for small single-level loops with low trip counts, as I observe them pretty frequently. I have no objection accepting this patch and improve the cost estimator separately.

Do you mean that you see such loops frequently with dynamically-small trip counts, or with static trip counts? I assume the small loops with (small) static trip counts will generally be unrolled.

In D34373#785367, @hfinkel wrote:

In D34373#784975, @twoh wrote:

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!

I know that we're currently missing opportunities for large vectorizable loops with low (static) trip counts. Smaller inner loops are also good candidates for unpredicated vectorization, but we may need to be a bit careful because of modeling inaccuracies and phase-ordering effects (e.g. if we don't vectorize a loop, then we'll end up unrolling it when the unroller runs).

I think this is a right approach, but concerned that the experimental results I shared on D32451 show that it is generally better to not to vectorize the low trip count loops. @Ayal, I wonder if you have any results that this patch actually improves the performance. Thanks!