[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-08 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment.

In D159339#4642210 , @v.g.vassilev 
wrote:

> In D159339#4642147 , @daltenty 
> wrote:
>
>> FYI: I tried to reproduce this on `powerpc64le-linux-gnu` at `-02` and 
>> didn't see it at this point, though I get a whole lot of other uninitialized 
>> reads, so there's definitely some general problems in this space in the LLVM 
>> codebase
>
> Are these also present in llvm16?

I do see a similar case in llvm16 yes:

  algrind --tool=memcheck --leak-check=full ./bin/clang-repl
  ==3583806== Memcheck, a memory error detector
  ==3583806== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
  ==3583806== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
  ==3583806== Command: ./bin/clang-repl
  ==3583806==
  clang-repl> #include 
  ==3583806== Conditional jump or move depends on uninitialised value(s)
  ==3583806==at 0x28D08F4: operator== 
(llvm-project/llvm/include/llvm/Support/Alignment.h:301)
  ==3583806==by 0x28D08F4: llvm::DataLayout::operator==(llvm::DataLayout 
const&) const (llvm-project/llvm/lib/IR/DataLayout.cpp:549)
  ==3583806==by 0x2AA31BB: operator!= 
(llvm-project/llvm/include/llvm/IR/DataLayout.h:233)
  ==3583806==by 0x2AA31BB: llvm::orc::LLJIT::applyDataLayout(llvm::Module&) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:927)
  ==3583806==by 0x2AA009F: operator() 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:754)
  ==3583806==by 0x2AA009F: withModuleDo<(lambda at 
/home/daltenty/llvm/dev/llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:754:28)>
 (llvm-project/llvm/include/llvm/ExecutionEngine/Orc/ThreadSafeModule.h:136)
  ==3583806==by 0x2AA009F: 
llvm::orc::LLJIT::addIRModule(llvm::IntrusiveRefCntPtr,
 llvm::orc::ThreadSafeModule) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:754)
  ==3583806==by 0x2AA0693: 
llvm::orc::LLJIT::addIRModule(llvm::orc::JITDylib&, 
llvm::orc::ThreadSafeModule) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:761)
  ==3583806==by 0x2AAB563: (anonymous 
namespace)::GenericLLVMIRPlatformSupport::setupJITDylib(llvm::orc::JITDylib&) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:253)
  ==3583806==by 0x2AA2B2F: GenericLLVMIRPlatformSupport 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:200)
  ==3583806==by 0x2AA2B2F: make_unique<(anonymous 
namespace)::GenericLLVMIRPlatformSupport, llvm::orc::LLJIT &> 
(unique_ptr.h:1065)
  ==3583806==by 0x2AA2B2F: 
llvm::orc::setUpGenericLLVMIRPlatform(llvm::orc::LLJIT&) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:947)
  ==3583806==by 0x2AA2273: 
llvm::orc::LLJIT::LLJIT(llvm::orc::LLJITBuilderState&, llvm::Error&) 
(llvm-project/llvm/lib/ExecutionEngine/Orc/LLJIT.cpp:911)
  ==3583806==by 0x26AE233: llvm::orc::LLJITBuilderSetters::create() 
(llvm-project/llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h:381)
  ==3583806==by 0x2F39D7F: 
clang::IncrementalExecutor::IncrementalExecutor(llvm::orc::ThreadSafeContext&, 
llvm::Error&, clang::TargetInfo const&) 
(llvm-project/clang/lib/Interpreter/IncrementalExecutor.cpp:40)
  ==3583806==by 0x2F33A47: make_unique 
(unique_ptr.h:1065)
  ==3583806==by 0x2F33A47: 
clang::Interpreter::Execute(clang::PartialTranslationUnit&) 
(llvm-project/clang/lib/Interpreter/Interpreter.cpp:223)
  ==3583806==by 0x26AE71B: 
clang::Interpreter::ParseAndExecute(llvm::StringRef) 
(llvm-project/clang/include/clang/Interpreter/Interpreter.h:68)
  ==3583806==by 0x26ADD6B: main 
(llvm-project/clang/tools/clang-repl/ClangRepl.cpp:127)
  ==3583806==

This definitely seems to be point to `DataLayout.FunctionPtrAlign`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-08 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

In D159339#4642147 , @daltenty wrote:

> FYI: I tried to reproduce this on `powerpc64le-linux-gnu` at `-02` and didn't 
> see it at this point, though I get a whole lot of other uninitialized reads, 
> so there's definitely some general problems in this space in the LLVM codebase

Are these also present in llvm16?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-08 Thread David Tenty via Phabricator via cfe-commits
daltenty added a comment.

FYI: I tried to reproduce this on `powerpc64le-linux-gnu` at `-02` and didn't 
see it at this point, though I get a whole lot of other uninitialized reads, so 
there's definitely some general problems in this space in the LLVM codebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-05 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a subscriber: Hahnfeld.
v.g.vassilev added a comment.

The issue goes away for non-CXXMethodDecls but still persists in the case where 
we process:

  CXXDestructorDecl 0x9ab0f48 

 col:11 implicit used ~shared_ptr 'void () noexcept' inline default
  `-CompoundStmt 0x9ac3ca0 

Here is a bt:

  #0  llvm::Value::getPointerAlignment (this=0x9dcbed8, DL=...) at 
/home/vvassilev/workspace/sources/llvm-project/llvm/lib/IR/Value.cpp:923
  #1  0x01b32b52 in 
clang::CodeGen::CodeGenModule::SetLLVMFunctionAttributesForDefinition 
(this=0x885e3c0, D=0x9ab0f48, F=0x9dcbed8)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:2389
  #2  0x01d40899 in clang::CodeGen::CodeGenModule::codegenCXXStructor 
(this=0x885e3c0, GD=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CGCXX.cpp:211
  #3  0x01bbffd5 in (anonymous 
namespace)::ItaniumCXXABI::emitCXXStructor (this=0x885f2f0, GD=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/ItaniumCXXABI.cpp:4453
  #4  0x01b36ffb in clang::CodeGen::CodeGenModule::EmitGlobalDefinition 
(this=this@entry=0x885e3c0, GD=..., GV=GV@entry=0x9dcbed8)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3893
  #5  0x01b2a9a0 in clang::CodeGen::CodeGenModule::EmitDeferred 
(this=this@entry=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3050
  #6  0x01b2a9cc in clang::CodeGen::CodeGenModule::EmitDeferred 
(this=this@entry=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:3056
  #7  0x01b285bb in clang::CodeGen::CodeGenModule::Release 
(this=0x885e3c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenModule.cpp:759
  #8  0x01882908 in (anonymous 
namespace)::CodeGeneratorImpl::HandleTranslationUnit (this=0x87c6430, Ctx=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/ModuleBuilder.cpp:287
  #9  0x0186e3a6 in clang::BackendConsumer::HandleTranslationUnit 
(this=0x87c6080, C=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/CodeGen/CodeGenAction.cpp:311
  #10 0x00f7bdc7 in clang::IncrementalParser::ParseOrWrapTopLevelDecl 
(this=this@entry=0x87603c0)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/IncrementalParser.cpp:303
  #11 0x00f7c760 in clang::IncrementalParser::Parse (this=0x87603c0, 
input=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/IncrementalParser.cpp:342
  #12 0x00f70c79 in clang::Interpreter::Parse (this=0x875d170, Code=...)
  at 
/home/vvassilev/workspace/sources/llvm-project/clang/lib/Interpreter/Interpreter.cpp:360
  #13 0x00e28b11 in Cpp::Interpreter::Parse (this=0x86e9cf0, Code=...)
  at 
/home/vvassilev/workspace/builds/scratch/cppyy/InterOp/include/clang/Interpreter/CppInterOpInterpreter.h:172
  #14 0x00e28b84 in Cpp::Interpreter::declare (this=0x86e9cf0, 
  input="\n#include \n\ntemplate\nclass 
derived_shared_ptr : public std::shared_ptr {};\ntemplate\n  
  class derived_unique_ptr : public std::unique_ptr {};\n\n   "..., PTU=0x0)

I am staring at the `Value::getPointerAlignment(const DataLayout )` and 
cannot find out what might be wrong. One particularity of the clang-repl based 
workflows is that it calls multiple times `StartModule`, which creates a new 
`llvm::Module` per partial translation unit.

cc: @Hahnfeld


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread Vassil Vassilev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG92246a9be0ba: [CodeGen] First check the kind and then the 
llvm::Function properties. (authored by v.g.vassilev).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2401,7 +2401,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2401,7 +2401,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

daltenty wrote:
> v.g.vassilev wrote:
> > daltenty wrote:
> > > Thanks for looking into this. 
> > > 
> > > It's not clear to me how this re-ordering ends up fixing things. Can you 
> > > clarify what the uninitialized value was in this expression?
> > > 
> > > 
> > The issue happens only in Release builds (RelWithDebInfo, too). From what I 
> > was able to see it is somewhere in `F->getPointerAlignment`. My assumption 
> > was that we cannot rely on the full properties of `F` to be set unless it 
> > is the declaration kind we expected (similar to checking if a something is 
> > a nullptr and then probing its members). Secondly the `isa` check is likely 
> > to be the less expensive check anyway.
> > 
> > I saw the issue yesterday and dug into it for a while. However, I decided 
> > to insert a "fix" before the release which is in few days since the `isa` 
> > seems to the faster check anyway.
> Thanks, yeah thats kind of what I expected.  `F->getPointerAlignment()` is 
> likely getting inlined into in to this callsite and we are inspecting 
> uninitialized properties of the DataLayout. The weird part is I don't see why 
> those properties of the DataLayout ever should be uninitialized, so I think 
> there might be something more broken underneath this.
> 
> That said, this is definitely better than before as you say, so let's go 
> ahead with this for the release and maybe I'll do some more digging in 
> `getPointerAlignment`.
Sounds good. For better or worse that workaround might make the issue more 
subtle to debug next time it appears...


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread David Tenty via Phabricator via cfe-commits
daltenty accepted this revision.
daltenty added a comment.
This revision is now accepted and ready to land.

LGTM as a workaround.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

v.g.vassilev wrote:
> daltenty wrote:
> > Thanks for looking into this. 
> > 
> > It's not clear to me how this re-ordering ends up fixing things. Can you 
> > clarify what the uninitialized value was in this expression?
> > 
> > 
> The issue happens only in Release builds (RelWithDebInfo, too). From what I 
> was able to see it is somewhere in `F->getPointerAlignment`. My assumption 
> was that we cannot rely on the full properties of `F` to be set unless it is 
> the declaration kind we expected (similar to checking if a something is a 
> nullptr and then probing its members). Secondly the `isa` check is likely to 
> be the less expensive check anyway.
> 
> I saw the issue yesterday and dug into it for a while. However, I decided to 
> insert a "fix" before the release which is in few days since the `isa` seems 
> to the faster check anyway.
Thanks, yeah thats kind of what I expected.  `F->getPointerAlignment()` is 
likely getting inlined into in to this callsite and we are inspecting 
uninitialized properties of the DataLayout. The weird part is I don't see why 
those properties of the DataLayout ever should be uninitialized, so I think 
there might be something more broken underneath this.

That said, this is definitely better than before as you say, so let's go ahead 
with this for the release and maybe I'll do some more digging in 
`getPointerAlignment`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

daltenty wrote:
> Thanks for looking into this. 
> 
> It's not clear to me how this re-ordering ends up fixing things. Can you 
> clarify what the uninitialized value was in this expression?
> 
> 
The issue happens only in Release builds (RelWithDebInfo, too). From what I was 
able to see it is somewhere in `F->getPointerAlignment`. My assumption was that 
we cannot rely on the full properties of `F` to be set unless it is the 
declaration kind we expected (similar to checking if a something is a nullptr 
and then probing its members). Secondly the `isa` check is likely to be the 
less expensive check anyway.

I saw the issue yesterday and dug into it for a while. However, I decided to 
insert a "fix" before the release which is in few days since the `isa` seems to 
the faster check anyway.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread David Tenty via Phabricator via cfe-commits
daltenty added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2389
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));

Thanks for looking into this. 

It's not clear to me how this re-ordering ends up fixing things. Can you 
clarify what the uninitialized value was in this expression?




Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159339/new/

https://reviews.llvm.org/D159339

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D159339: [urgent][CodeGen] First check the kind and then the llvm::Function properties.

2023-09-01 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev created this revision.
v.g.vassilev added reviewers: rjmccall, daltenty, aaron.ballman.
Herald added a subscriber: pengfei.
Herald added a project: All.
v.g.vassilev requested review of this revision.
Herald added a subscriber: wangpc.

This patch fixes valgrind reports from downstream consumers about conditional 
jump over uninitialised.

  [ RUN  ] ScopeReflectionTest.IsComplete
  ==987150== Conditional jump or move depends on uninitialised value(s)
  ==987150==at 0x1E1128F: 
clang::CodeGen::CodeGenModule::SetLLVMFunctionAttributesForDefinition(clang::Decl
 const*, llvm::Function*) (CodeGenModule.cpp:2391)
  ==987150==by 0x1E4F181: 
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) (CodeGenModule.cpp:5669)
  ==987150==by 0x1E4A194: 
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::GlobalDecl, 
llvm::GlobalValue*) (CodeGenModule.cpp:3909)
  ==987150==by 0x1E4A752: 
clang::CodeGen::CodeGenModule::EmitGlobal(clang::GlobalDecl) 
(CodeGenModule.cpp:3649)
  ==987150==by 0x1E532F5: 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) [clone .part.0] 
(CodeGenModule.cpp:6563)
  ==987150==by 0x1B0BEDD: (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) 
(ModuleBuilder.cpp:190)
  ==987150==by 0x1AEA47B: 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
(CodeGenAction.cpp:235)
  ==987150==by 0x101B02F: 
clang::IncrementalASTConsumer::HandleTopLevelDecl(clang::DeclGroupRef) 
(IncrementalParser.cpp:52)
  ==987150==by 0x101ED93: 
clang::IncrementalParser::ParseOrWrapTopLevelDecl() (IncrementalParser.cpp:276)
  ==987150==by 0x101FBBC: clang::IncrementalParser::Parse(llvm::StringRef) 
(IncrementalParser.cpp:342)
  ==987150==by 0x100E104: clang::Interpreter::Parse(llvm::StringRef) 
(Interpreter.cpp:360)
  ==987150==by 0xE734C0: Cpp::Interpreter::Parse(llvm::StringRef) 
(CppInterOpInterpreter.h:172)
  ==987150==  Uninitialised value was created by a heap allocation
  ==987150==at 0x844BE63: operator new(unsigned long) (in 
/usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
  ==987150==by 0x1B0C882: StartModule (ModuleBuilder.cpp:139)
  ==987150==by 0x1B0C882: 
clang::CodeGenerator::StartModule(llvm::StringRef, llvm::LLVMContext&) 
(ModuleBuilder.cpp:360)
  ==987150==by 0x101C4AF: clang::IncrementalParser::GenModule() 
(IncrementalParser.cpp:372)
  ==987150==by 0x101FC0E: clang::IncrementalParser::Parse(llvm::StringRef) 
(IncrementalParser.cpp:362)
  ==987150==by 0x100E104: clang::Interpreter::Parse(llvm::StringRef) 
(Interpreter.cpp:360)
  ==987150==by 0x100E243: 
clang::Interpreter::create(std::unique_ptr >) (Interpreter.cpp:279)
  ==987150==by 0xF2131A: compat::createClangInterpreter(std::vector >&) (Compatibility.h:123)
  ==987150==by 0xF22AB9: Cpp::Interpreter::Interpreter(int, char const* 
const*, char const*, std::vector, 
std::allocator > > const&, void*, 
bool) (CppInterOpInterpreter.h:146)
  ==987150==by 0xF1827A: CreateInterpreter (CppInterOp.cpp:2494)
  ==987150==by 0xECFA0E: 
TestUtils::GetAllTopLevelDecls(std::__cxx11::basic_string, std::allocator > const&, 
std::vector >&, bool) (Utils.cpp:23)
  ==987150==by 0xE9CB85: ScopeReflectionTest_IsComplete_Test::TestBody() 
(ScopeReflectionTest.cpp:71)
  ==987150==by 0xF0ED0C: void 
testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) (in 
/home/vvassilev/workspace/builds/scratch/cppyy/InterOp/build-with-clang-repl-release/unittests/CppInterOp/CppInterOpTests)
  ==987150== 


Repository:
  rC Clang

https://reviews.llvm.org/D159339

Files:
  clang/lib/CodeGen/CodeGenModule.cpp


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2386,7 +2386,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2), F->getAlign().valueOrOne()));
   }
 


Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -2386,7 +2386,7 @@
   // functions. If the current target's C++ ABI requires this and this is a
   // member function, set its alignment accordingly.
   if (getTarget().getCXXABI().areMemberFunctionsAligned()) {
-if (F->getPointerAlignment(getDataLayout()) < 2 && isa(D))
+if (isa(D) && F->getPointerAlignment(getDataLayout()) < 2)
   F->setAlignment(std::max(llvm::Align(2),