This is an archive of the discontinued LLVM Phabricator instance.

Differential D72759

OpenMP breaks with PerformPendingInstatiations() already in the PCH
AbandonedPublic

Authored by llunak on Jan 15 2020, 4:07 AM.

Details

Reviewers
ABataev
jdoerfert
Summary

This is OpenMP-related part split off from https://reviews.llvm.org/D69585 .

When the referenced change is used to do template instatiations already in the PCH, OpenMP/declare_target_codegen.cpp test fails because some virtuals of template classes are not emitted. This is fixed by this patch. I do not understand the purpose of that code (neither I have any clue about OpenMP), but it was introduced in d2c1dd5902782fb773c64dbf4e0b529aa4044b05 , which also changed this very test, and the change makes the test pass again.

Diff Detail

Event Timeline

llunak created this revision.Jan 15 2020, 4:07 AM
Herald added a reviewer: jdoerfert. · View Herald TranscriptJan 15 2020, 4:07 AM
Herald added a subscriber: guansong. · View Herald Transcript
ABataev added a comment.Jan 15 2020, 6:41 AM

Fails in what way?

llunak added a comment.Jan 15 2020, 8:10 AM
In D72759#1821759, @ABataev wrote:

Fails in what way?

/home/seli/build/src/llvm/clang/test/OpenMP/declare_target_codegen.cpp:234:15: error: CHECK-DAG: expected string not found in input
// CHECK-DAG: define {{.*}}void @__omp_offloading_{{.*}}virtual_foo{{.*}}_l[[@LINE-25]]()

That is the 4th RUN command in the file. The diff of its output between the 3rd command run without and with -pch-instantiate-templates is this (minus some irrelevant reordering):

--- a2.txt	2020-01-15 16:58:52.300810143 +0100
+++ a1.txt	2020-01-15 17:01:11.469450622 +0100
@@ -80,10 +80,6 @@ $_ZTI4BakeIiE = comdat any
 @.omp_offloading.entry.__omp_offloading_10303_21d885__Z4baz5v_l169 = weak constant %struct.__tgt_offload_entry { i8* bitcast (void (i64)* @__omp_offloading_10303_21d885__Z4baz5v_l169 to i8*), i8* getelementptr inbounds ([44 x i8], [44 x i8]* @.omp_offloading.entry_name.10, i32 0, i32 0), i64 0, i32 0, i32 0 }, section "omp_offloading_entries", align 1
 @.omp_offloading.entry_name.11 = internal unnamed_addr constant [16 x i8] c"out_decl_target\00"
 @.omp_offloading.entry.out_decl_target = weak constant %struct.__tgt_offload_entry { i8* bitcast (i32* @out_decl_target to i8*), i8* getelementptr inbounds ([16 x i8], [16 x i8]* @.omp_offloading.entry_name.11, i32 0, i32 0), i64 4, i32 0, i32 0 }, section "omp_offloading_entries", align 1
-@.omp_offloading.entry_name.12 = internal unnamed_addr constant [59 x i8] c"__omp_offloading_10303_21d885__ZN1BIiE11virtual_fooEv_l209\00"
-@.omp_offloading.entry.__omp_offloading_10303_21d885__ZN1BIiE11virtual_fooEv_l209 = weak constant %struct.__tgt_offload_entry { i8* bitcast (void ()* @__omp_offloading_10303_21d885__ZN1BIiE11virtual_fooEv_l209 to i8*), i8* getelementptr inbounds ([59 x i8], [59 x i8]* @.omp_offloading.entry_name.12, i32 0, i32 0), i64 0, i32 0, i32 0 }, section "omp_offloading_entries", align 1
-@.omp_offloading.entry_name.13 = internal unnamed_addr constant [54 x i8] c"__omp_offloading_10303_21d885__ZN1CIiE7emittedEv_l224\00"
-@.omp_offloading.entry.__omp_offloading_10303_21d885__ZN1CIiE7emittedEv_l224 = weak constant %struct.__tgt_offload_entry { i8* bitcast (void ()* @__omp_offloading_10303_21d885__ZN1CIiE7emittedEv_l224 to i8*), i8* getelementptr inbounds ([54 x i8], [54 x i8]* @.omp_offloading.entry_name.13, i32 0, i32 0), i64 0, i32 0, i32 0 }, section "omp_offloading_entries", align 1
 @llvm.used = appending global [2 x i8*] [i8* bitcast (void ()* @__omp_offloading__10303_21d885_globals_l133_ctor to i8*), i8* bitcast (void ()* @__omp_offloading__10303_21d885_stat_l134_ctor to i8*)], section "llvm.metadata"
 @llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (%struct.S** @._ZL4stat.ref to i8*)], section "llvm.metadata"
 ; Function Attrs: noinline nounwind optnone
@@ -271,16 +267,6 @@ entry:
   call void @_ZdlPv(i8* %0) #7
   ret void
 }
-; Function Attrs: noinline norecurse nounwind optnone
-define weak void @__omp_offloading_10303_21d885__ZN1BIiE11virtual_fooEv_l209() #2 {
-entry:
-  ret void
-}
-; Function Attrs: noinline norecurse nounwind optnone
-define weak void @__omp_offloading_10303_21d885__ZN1CIiE7emittedEv_l224() #2 {
-entry:
-  ret void
-}
 ; Function Attrs: noinline nounwind optnone
 define linkonce_odr hidden void @_ZN1SC2Ei(%struct.S* %this, i32 signext %a) unnamed_addr #0 comdat align 2 {
 entry:
ABataev added a comment.Jan 15 2020, 10:51 AM

Just maybe, need to add TUKind == TU_Complete instead. Will investigate it and make a patch.

ABataev added a comment.Jan 15 2020, 3:46 PM

Must be fixed in b841b9e96e605bed5a1f9b846a07aae88c65ce02

llunak abandoned this revision.Jan 16 2020, 4:55 AM
In D72759#1823062, @ABataev wrote:

Must be fixed in b841b9e96e605bed5a1f9b846a07aae88c65ce02

I can confirm. Thank you.

llunak added a comment.Jan 16 2020, 6:41 AM

Could you please still provide input in https://reviews.llvm.org/D69585? The patch still fails in 21 OpenMP tests simply because of compiler output getting reordered, which seems to require extensive reorganizing of the tests, and I don't want to do extensive (and tedious) changes there without your input.

Revision Contents

PathSize
clang/
lib/
Sema/
2 lines

Diff 238216

clang/lib/Sema/SemaDeclCXX.cpp

Context not available.
return; return;
// Do not mark as used if compiling for the device outside of the target // Do not mark as used if compiling for the device outside of the target
// region. // region.
if (LangOpts.OpenMP && LangOpts.OpenMPIsDevice && if (TUKind != TU_Prefix && LangOpts.OpenMP && LangOpts.OpenMPIsDevice &&
!isInOpenMPDeclareTargetContext() && !isInOpenMPDeclareTargetContext() &&
!isInOpenMPTargetExecutionDirective()) { !isInOpenMPTargetExecutionDirective()) {
if (!DefinitionRequired) if (!DefinitionRequired)
Context not available.