In D69585#1825252, @llunak wrote:

In D69585#1825133, @ABataev wrote:

I thought you were going to add an option or a flag to control the behavior? If so, just provide an option in tests to avoid triggering of the new behavior (except for declare_target... test and those 2 you modified already) and that's it.

It's not included in the latest version of the patch. As written above, I'm reasonably sure I was mistaken about the need for a flag, and it should be ok to simply do the change unconditionally. I can put the flag back just for the purpose of the tests if you want, that'd certainly make handling of the tests trivial, but then the tests wouldn't really test "normal" PCHs, so do you really want that?

In D69585#1823969, @llunak wrote:

In D69585#1821831, @aganea wrote:

What is the error?

I take that part back, actually. I don't quite remember anymore what exactly I did in October, but if I now revert the PCH tweaks in LibreOffice I did to avoid the error, the compilation fails even without the patch or with GCC. So I assume what really happened was that code changes triggered the error and I incorrectly assumed it was because of my patch. So, unless proven otherwise, I take it that my patch is actually technically correct without causing any code regressions (the OpenMP problem has just been fixed).

What remains is those 22 tests which fail because moving the instantiations reorders the code that is expected by FileCheck. This patch handles 2 of them, and it's already rather tedious (the OpenMP tests are large). Is this really the way to handle them, or does somebody have a better idea?

Hmm, we still need to find some basic decl to remap it successfully at the codegen. Not sure that we'll be able to support it in full. It would be good to investigate how we can handle them at the codegen.

Not sure that we need to dig that deep. Maybe just keep thw original analysis for the known expressions and map all other lvalues without some extra analysis, just like DeclRefExpr nodes?

LG.

Must be fixed in b841b9e96e605bed5a1f9b846a07aae88c65ce02

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

LG

Fails in what way?

Need a test

I don't see any crashes in OpenMP tests, other tests just must be updated by the author, if this patch will ever going to be landed. I don't think it is a good idea to have a not fully functional implementation, even guarded by the option.

In D72576#1820258, @tianshilei1992 wrote:

In D72576#1820230, @ABataev wrote:

Could you provide full diff of your modificaions?

This is exactly the full diff.

Could you provide full diff of your modificaions?

In D72472#1817787, @tianshilei1992 wrote:

I got a question: since there is no target region in the shared library at all, why is desc->NumDeviceImages = 1 where desc is the one in the shared library?

In D72576#1816159, @tianshilei1992 wrote:

In D72576#1816111, @ABataev wrote:

"Revert" is wrong here since one of the tests will definitely fail after this. Prepare a patch with the proper fix.

Yep. I got your point. Didn't notice what you tried to fix before. Will update correspondingly.

"Revert" is wrong here since one of the tests will definitely fail after this. Prepare a patch with the proper fix.

LG

Rebase + address comments.

In D30686#1795400, @RKSimon wrote:

@ABataev Would it be worth resurrecting this or starting again? We have a similar test case in https://bugs.llvm.org/show_bug.cgi?id=44008

current:

%L0 = extractelement <2 x float> %1, i32 0
%L1 = extractelement <2 x float> %1, i32 1
%Mul0 = fmul float %L0, 2.000000e+00
%Mul1 = fmul float %L1, 2.000000e+00

better:

%Mul = fmul <2 x float> %1, <2.000000e+00, 2.000000e+00>
%L0 = extractelement <2 x float> %Mul, i32 0
%L1 = extractelement <2 x float> %Mul, i32 1

In D57779#1795146, @dtemirbulatov wrote:

Try to restore the context.

In D57779#1794675, @dtemirbulatov wrote:

Rebased after committing the first part as NFC, it looks like resolved all remarks.

Removed check for isArrow() + added requested test.

Fix + rebase.

Fix + rebase.

LG

Fixed the bug + rebase.

Looks much better in general, need to resolve last one issue with number of elements/size and you're ready.

In D70258#1788825, @rjmccall wrote:

So it's never that OpenMP has things that need to be finalized before exiting (e.g. if something in frontend-emitted code throws an exception), it's just that OpenMP might need to generate its own control flow that breaks through arbitrary scopes in the frontend?

I would really hope that the OpenMP implementation in Clang would've used Clang's cleanup stack rather than inventing its own mechanism.

The fixed patch. Several codegen tests require some adjustment.

diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index afe0f1a..ecb0fb2 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -919,6 +919,11 @@ static const Expr *getExprAsWritten(const Expr *E) {

In D71241#1788003, @jdoerfert wrote:

In D71241#1787652, @hfinkel wrote:

In D71241#1787571, @ABataev wrote:

In D71241#1787265, @hfinkel wrote:

In D71241#1786959, @jdoerfert wrote:

In D71241#1786530, @ABataev wrote:

Most probably, we can use this solution without adding a new expression. DeclRefExpr class can contain 2 decls: FoundDecl and the Decl being used. You can use FoundDecl to point to the original function and used decl to point to the function being called in this context. But at first, we need to be sure that we can handle all corner cases correctly.

What new expression are you talking about?

To be clear, I believe he's talking about the new expression that I proposed we add in order to represent this kind of call. If that's not needed, and we can use the FoundDecl/Decl pair for that purpose, that should also be considered.

This solution already does point to both declarations, as shown here: https://reviews.llvm.org/D71241#1782504

Exactly. We need to check if the MemberRefExpr can do this too to handle member functions correctly.
And need to wait for opinion from Richard Smith about the design. We need to be sure that it won't break compatibility with C/C++ in some corner cases. Design bugs are very hard to solve and I want to be sure that we don't miss anything. And we provide full compatibility with both C and C++.

We do need to be careful here. For cases with FoundDecl != Decl, I think that the typo-correction cases look like they probably work, but there are a few places where we make semantic decisions based on the mismatch, such as:

In SemaTemplate.cpp below line 512, we have (this is in C++03-specific code):

} else if (!Found.isSuppressingDiagnostics()) {
  //   - if the name found is a class template, it must refer to the same
  //     entity as the one found in the class of the object expression,
  //     otherwise the program is ill-formed.
  if (!Found.isSingleResult() ||
      getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
          OuterTemplate->getCanonicalDecl()) {
    Diag(Found.getNameLoc(),
         diag::ext_nested_name_member_ref_lookup_ambiguous)

and in SemaExpr.cpp near line 2783, we have:

// If we actually found the member through a using declaration, cast
// down to the using declaration's type.
//
// Pointer equality is fine here because only one declaration of a
// class ever has member declarations.
if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
  assert(isa<UsingShadowDecl>(FoundDecl));
  QualType URecordType = Context.getTypeDeclType(
                         cast<CXXRecordDecl>(FoundDecl->getDeclContext()));

Could you specify what behavior you expect, or what the test casees would look like?

For the record:
OpenMP basically says, if you have a call to a (base)function that has variants with contexts that match at the call site, call the variant with the highest score. The variants are specified by a variant-func-id, which is a base language identifier or C++ template-id. For C++, the variant declaration is identified by *performing the base language lookup rules on the variant-func-id with arguments that correspond to the base function argument types*.