Hello everyone, This is a rather ancient code with Chris's name all over it, so I naturally feel humbled :) I see a conceptual issue in lib/Transforms/IPO/GlobalOpt.cpp with several optimizations that create a copy of GlobalVariable without copying attributes from the original one. Consider this one: http://llvm.org/doxygen/GlobalOpt_8cpp_source.html static bool TryToShrinkGlobalToBoolean(GlobalVariable *GV, Constant *OtherVal) { ... // Create the new global, initializing it to false. GlobalVariable *NewGV = new GlobalVariable(Type::getInt1Ty(GV->getContext()), false, GlobalValue::InternalLinkage, ConstantInt::getFalse(GV->getContext()), GV->getName()+".b", GV->getThreadLocalMode(), GV->getType()->getAddressSpace()); GV->getParent()->getGlobalList().insert(GV, NewGV); ... // Retain the name of the old global variable. People who are debugging their // programs may expect these variables to be named the same. NewGV->takeName(GV); GV->eraseFromParent(); What I do not see - the section information from the original GV is never copied to the NewGV, so this test would be failing for me: ; RUN: opt < %s -S -globalopt -instcombine | FileCheck %s ;; check that global opt turns integers into bools and keeps section info. @G = internal addrspace(1) global i32 0, section ".foo" ; CHECK: @G ; CHECK: addrspace(1) ; CHECK: global i1 false ; CHECK: section ".foo" define void @set1() { store i32 0, i32 addrspace(1)* @G ; CHECK: store i1 false ret void } define void @set2() { store i32 1, i32 addrspace(1)* @G ; CHECK: store i1 true ret void } define i1 @get() { ; CHECK-LABEL: @get( %A = load i32, i32 addrspace(1) * @G %C = icmp slt i32 %A, 2 ret i1 %C ; CHECK: ret i1 true } Now the proverbial question - is this a bug or a feature? ...and if it was meant to be done that way then why... Beware, that this is the way it is done throughout the file, not just in this specific instance. Thanks a lot. Sergei Larin --- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On Sep 18, 2015, at 10:45 AM, Sergei Larin <slarin at codeaurora.org> wrote:> What I do not see - the section information from the original GV is never copied to the NewGV, so this test would be failing for me: > Now the proverbial question - is this a bug or a feature? ...and if it was meant to be done that way then why…Historically speaking, this optimization probably predated the ability to put a section on a global. That said, I think that this is a feature: The sorts of optimizations that globalopt does are extremely transformative to the underlying global variable, and whatever reason the global was assigned to a section in the first place is probably not going to work with the transformed value. If you’re having a problem with this, it is probably best to just make globalopt more conservative: have it refuse to transform a global that has any explicitly assigned section. -Chris
Chris, Thanks for the clarification... at least no bug report is due... and I am glad that I've asked. In my case these transformations are rather useful and forcing them to copy original global variable section is making them compatible with our (rather important) use case, so I guess I will have to fix it locally. Nevertheless if someone else would have a similar issue - I would be happy to patch it. Sergei --- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation> -----Original Message----- > From: Chris Lattner [mailto:clattner at apple.com] > Sent: Sunday, September 20, 2015 5:28 PM > To: Sergei Larin > Cc: llvm-dev at lists.llvm.org; Lang Hames > Subject: Re: GlobalOPT and sections > > On Sep 18, 2015, at 10:45 AM, Sergei Larin <slarin at codeaurora.org> wrote: > > What I do not see - the section information from the original GV is never > copied to the NewGV, so this test would be failing for me: > > Now the proverbial question - is this a bug or a feature? ...and if it was > meant to be done that way then why… > > Historically speaking, this optimization probably predated the ability to put a > section on a global. > > That said, I think that this is a feature: The sorts of optimizations that > globalopt does are extremely transformative to the underlying global > variable, and whatever reason the global was assigned to a section in the first > place is probably not going to work with the transformed value. > > If you’re having a problem with this, it is probably best to just make globalopt > more conservative: have it refuse to transform a global that has any explicitly > assigned section. > > -Chris