Re: r328040 - Set dso_local on string literals.

[Rafael Avila de Espindola via cfe-commits]

Joerg Sonnenberger via cfe-commits  writes:

> On Tue, Mar 20, 2018 at 08:42:55PM -, Rafael Espindola via cfe-commits 
> wrote:
>> Author: rafael
>> Date: Tue Mar 20 13:42:55 2018
>> New Revision: 328040
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=328040=rev
>> Log:
>> Set dso_local on string literals.
>
> I wonder if unnamed_addr shouldn't imply that in general?

I don't think so. For example, a language where functions cannot be
compared for equality could mark all functions unnamed_addr. It could
still support interposition, which requires that they are not dso_local.

Cheers,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r328006 - [NVPTX] Make tensor load/store intrinsics overloaded.

[Rafael Avila de Espindola via cfe-commits]

Sorry, I was just missing "git pull" in the llvm repo.

Cheers,
Rafael

Artem Belevich  writes:

> Thanks for the heads up.
>
> Which buildbot shows the failure? I don't see the failure on the cuda
> buildbot, nor do I see it on my machine locally.
>
> It may be due to llvm/clang being out of sync. The commit had changes for
> both sides and if clang and llvm are out of sync, you may see this kind of
> error.
>
> --Artem
>
>
> On Tue, Mar 20, 2018 at 1:33 PM Rafael Avila de Espindola <
> rafael.espind...@gmail.com> wrote:
>
>> With this clang/test/CodeGen/builtins-nvptx-sm_70.cu is crashing:
>>
>> lib/IR/Instructions.cpp:299: void
>> llvm::CallInst::init(llvm::FunctionType *, llvm::Value *,
>> ArrayRef, ArrayRef, const
>> llvm::Twine &): Assertion `(i >= FTy->getNumParams()||
>> FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with
>> a bad signature!"' failed.
>>
>> Cheers,
>> Rafael
>>
>>
>> Artem Belevich via cfe-commits  writes:
>>
>> > Author: tra
>> > Date: Tue Mar 20 10:18:59 2018
>> > New Revision: 328006
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=328006=rev
>> > Log:
>> > [NVPTX] Make tensor load/store intrinsics overloaded.
>> >
>> > This way we can support address-space specific variants without
>> explicitly
>> > encoding the space in the name of the intrinsic. Less intrinsics to deal
>> with ->
>> > less boilerplate.
>> >
>> > Added a bit of tablegen magic to match/replace an intrinsics with a
>> pointer
>> > argument in particular address space with the space-specific instruction
>> > variant.
>> >
>> > Updated tests to use non-default address spaces.
>> >
>> > Differential Revision: https://reviews.llvm.org/D43268
>> >
>> > Modified:
>> > cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=328006=328005=328006=diff
>> >
>> ==
>> > --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Mar 20 10:18:59 2018
>> > @@ -10527,8 +10527,7 @@ Value *CodeGenFunction::EmitNVPTXBuiltin
>> >llvm_unreachable("Unexpected builtin ID.");
>> >  }
>> >  Value *Result =
>> > -Builder.CreateCall(CGM.getIntrinsic(IID),
>> > -   {Builder.CreatePointerCast(Src, VoidPtrTy),
>> Ldm});
>> > +Builder.CreateCall(CGM.getIntrinsic(IID, Src->getType()), {Src,
>> Ldm});
>> >
>> >  // Save returned values.
>> >  for (unsigned i = 0; i < NumResults; ++i) {
>> > @@ -10567,10 +10566,9 @@ Value *CodeGenFunction::EmitNVPTXBuiltin
>> >  default:
>> >llvm_unreachable("Unexpected builtin ID.");
>> >  }
>> > -Function *Intrinsic = CGM.getIntrinsic(IID);
>> > +Function *Intrinsic = CGM.getIntrinsic(IID, Dst->getType());
>> >  llvm::Type *ParamType =
>> Intrinsic->getFunctionType()->getParamType(1);
>> > -SmallVector Values;
>> > -Values.push_back(Builder.CreatePointerCast(Dst, VoidPtrTy));
>> > +SmallVector Values = {Dst};
>> >  for (unsigned i = 0; i < NumResults; ++i) {
>> >Value *V = Builder.CreateAlignedLoad(
>> >Builder.CreateGEP(Src.getPointer(),
>> llvm::ConstantInt::get(IntTy, i)),
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> -- 
> --Artem Belevich
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r328006 - [NVPTX] Make tensor load/store intrinsics overloaded.

[Rafael Avila de Espindola via cfe-commits]

With this clang/test/CodeGen/builtins-nvptx-sm_70.cu is crashing:

lib/IR/Instructions.cpp:299: void
llvm::CallInst::init(llvm::FunctionType *, llvm::Value *,
ArrayRef, ArrayRef, const
llvm::Twine &): Assertion `(i >= FTy->getNumParams()||
FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with
a bad signature!"' failed.

Cheers,
Rafael


Artem Belevich via cfe-commits  writes:

> Author: tra
> Date: Tue Mar 20 10:18:59 2018
> New Revision: 328006
>
> URL: http://llvm.org/viewvc/llvm-project?rev=328006=rev
> Log:
> [NVPTX] Make tensor load/store intrinsics overloaded.
>
> This way we can support address-space specific variants without explicitly
> encoding the space in the name of the intrinsic. Less intrinsics to deal with 
> ->
> less boilerplate.
>
> Added a bit of tablegen magic to match/replace an intrinsics with a pointer
> argument in particular address space with the space-specific instruction
> variant.
>
> Updated tests to use non-default address spaces.
>
> Differential Revision: https://reviews.llvm.org/D43268
>
> Modified:
> cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=328006=328005=328006=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Mar 20 10:18:59 2018
> @@ -10527,8 +10527,7 @@ Value *CodeGenFunction::EmitNVPTXBuiltin
>llvm_unreachable("Unexpected builtin ID.");
>  }
>  Value *Result =
> -Builder.CreateCall(CGM.getIntrinsic(IID),
> -   {Builder.CreatePointerCast(Src, VoidPtrTy), Ldm});
> +Builder.CreateCall(CGM.getIntrinsic(IID, Src->getType()), {Src, 
> Ldm});
>  
>  // Save returned values.
>  for (unsigned i = 0; i < NumResults; ++i) {
> @@ -10567,10 +10566,9 @@ Value *CodeGenFunction::EmitNVPTXBuiltin
>  default:
>llvm_unreachable("Unexpected builtin ID.");
>  }
> -Function *Intrinsic = CGM.getIntrinsic(IID);
> +Function *Intrinsic = CGM.getIntrinsic(IID, Dst->getType());
>  llvm::Type *ParamType = Intrinsic->getFunctionType()->getParamType(1);
> -SmallVector Values;
> -Values.push_back(Builder.CreatePointerCast(Dst, VoidPtrTy));
> +SmallVector Values = {Dst};
>  for (unsigned i = 0; i < NumResults; ++i) {
>Value *V = Builder.CreateAlignedLoad(
>Builder.CreateGEP(Src.getPointer(), llvm::ConstantInt::get(IntTy, 
> i)),
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r327738 - [MS] Don't escape MS C++ names with \01

[Rafael Avila de Espindola via cfe-commits]

Thanks!

Reid Kleckner via cfe-commits  writes:

> Author: rnk
> Date: Fri Mar 16 13:36:49 2018
> New Revision: 327738
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327738=rev
> Log:
> [MS] Don't escape MS C++ names with \01
>
> It is not needed after LLVM r327734. Now it will be easier to copy-paste
> IR symbol names from Clang.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D42217: Set Module Metadata "AvoidPLT" when -fno-plt is used.

[Rafael Avila de Espindola via cfe-commits]

Sriraman Tallam via Phabricator  writes:
> +  if (CodeGenOpts.NoPLT) {
> +getModule().setAvoidPLT();
> +  }
> +

You don't need the {}.

LGTM with that.

Cheers,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D43514: Start settinng dso_local for COFF

[Rafael Avila de Espindola via cfe-commits]

Eric Christopher via Phabricator  writes:

> echristo added inline comments.
>
>
> 
> Comment at: lib/CodeGen/CodeGenModule.h:728
> +  /// This must be called after dllimport/dllexport is set.
> +  /// FIXME: should this set dllimport/dllexport instead?
>void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;
> 
> Agreed? Maybe? Should the rest of the properties from above each call be set 
> in this function?
>
> Since you've got the decl it might be best to get as many of them in one 
> place as possible?

Very likely yes.

One think I have locally is a very ugly patch that sets dso_local all
over the place in clang until every GV in coff is dso_local or
dllimport.

My idea is to cleanup it bit by bit once the initial patch (this one) is
in.

I can try moving dllimport/dllexport setting to setGVProperties as the
first followup patch if you agree.

Cheers,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D43362: Simplify setting dso_local. NFC.

[Rafael Avila de Espindola via cfe-commits]

ping

Rafael Avila de Espindola via Phabricator 
writes:

> espindola created this revision.
> espindola added a reviewer: rnk.
>
> The value of dso_local can be computed from just IR properties and global 
> information (object file type, command line options, etc).
>
> With this patch we no longer pass in the Decl. It was almost unused and 
> making it fully unused guarantees that dso_local is consistent with the rest 
> of the IR.
>
>
> https://reviews.llvm.org/D43362
>
> Files:
>   lib/CodeGen/CodeGenModule.cpp
>   lib/CodeGen/CodeGenModule.h
>   lib/CodeGen/ItaniumCXXABI.cpp
>
>
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -3214,10 +3214,10 @@
>  llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
>  
>TypeName->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(TypeName, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(TypeName);
>  
>GV->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(GV, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(GV);
>  
>if (CGM.getTriple().isWindowsItaniumEnvironment()) {
>  auto RD = Ty->getAsCXXRecordDecl();
> Index: lib/CodeGen/CodeGenModule.h
> ===
> --- lib/CodeGen/CodeGenModule.h
> +++ lib/CodeGen/CodeGenModule.h
> @@ -721,7 +721,7 @@
>/// Set the visibility for the given LLVM GlobalValue.
>void setGlobalVisibility(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> -  void setDSOLocal(llvm::GlobalValue *GV, const NamedDecl *D) const;
> +  void setDSOLocal(llvm::GlobalValue *GV) const;
>  
>void setGVProperties(llvm::GlobalValue *GV, const NamedDecl *D) const;
>  
> Index: lib/CodeGen/CodeGenModule.cpp
> ===
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -716,7 +716,7 @@
>  }
>  
>  static bool shouldAssumeDSOLocal(const CodeGenModule ,
> - llvm::GlobalValue *GV, const NamedDecl *D) {
> + llvm::GlobalValue *GV) {
>const llvm::Triple  = CGM.getTriple();
>// Only handle ELF for now.
>if (!TT.isOSBinFormatELF())
> @@ -746,31 +746,30 @@
>  return false;
>  
>// If we can use copy relocations we can assume it is local.
> -  if (auto *VD = dyn_cast(D))
> -if (VD->getTLSKind() == VarDecl::TLS_None &&
> +  if (auto *Var = dyn_cast(GV))
> +if (!Var->isThreadLocal() &&
>  (RM == llvm::Reloc::Static || CGOpts.PIECopyRelocations))
>return true;
>  
>// If we can use a plt entry as the symbol address we can assume it
>// is local.
>// FIXME: This should work for PIE, but the gold linker doesn't support it.
> -  if (isa(D) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
> +  if (isa(GV) && !CGOpts.NoPLT && RM == llvm::Reloc::Static)
>  return true;
>  
>// Otherwise don't assue it is local.
>return false;
>  }
>  
> -void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV,
> -const NamedDecl *D) const {
> -  if (shouldAssumeDSOLocal(*this, GV, D))
> +void CodeGenModule::setDSOLocal(llvm::GlobalValue *GV) const {
> +  if (shouldAssumeDSOLocal(*this, GV))
>  GV->setDSOLocal(true);
>  }
>  
>  void CodeGenModule::setGVProperties(llvm::GlobalValue *GV,
>  const NamedDecl *D) const {
>setGlobalVisibility(GV, D);
> -  setDSOLocal(GV, D);
> +  setDSOLocal(GV);
>  }
>  
>  static llvm::GlobalVariable::ThreadLocalMode GetLLVMTLSModel(StringRef S) {
> @@ -2753,14 +2752,15 @@
>  GV->setAlignment(getContext().getDeclAlign(D).getQuantity());
>  
>  setLinkageForGV(GV, D);
> -setGVProperties(GV, D);
>  
>  if (D->getTLSKind()) {
>if (D->getTLSKind() == VarDecl::TLS_Dynamic)
>  CXXThreadLocals.push_back(D);
>setTLSMode(GV, *D);
>  }
>  
> +setGVProperties(GV, D);
> +
>  // If required by the ABI, treat declarations of static data members with
>  // inline initializers as definitions.
>  if (getContext().isMSStaticDataMemberInlineDefinition(D)) {
>
>
> Index: lib/CodeGen/ItaniumCXXABI.cpp
> ===
> --- lib/CodeGen/ItaniumCXXABI.cpp
> +++ lib/CodeGen/ItaniumCXXABI.cpp
> @@ -3214,10 +3214,10 @@
>  llvmVisibility = CodeGenModule::GetLLVMVisibility(Ty->getVisibility());
>  
>TypeName->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(TypeName, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(TypeName);
>  
>GV->setVisibility(llvmVisibility);
> -  CGM.setDSOLocal(GV, Ty->getAsCXXRecordDecl());
> +  CGM.setDSOLocal(GV);
>  
>if (CGM.getTriple().isWindowsItaniumEnvironment()) {
>  auto RD = Ty->getAsCXXRecordDecl();
> Index: lib/CodeGen/CodeGenModule.h
> 

Re: [PATCH] D41318: Start setting dso_local in clang

[Rafael Avila de Espindola via cfe-commits]

Sean Fertile via Phabricator  writes:

> sfertile added inline comments.
>
>
> 
> Comment at: clang/lib/CodeGen/CodeGenModule.cpp:750
> +  // If we can use a plt entry as the symbol address we can assume it
> +  // is local.
> +  if (isa(D) && !CGOpts.NoPLT)
> 
> I don't think this is the case. I think this would break ppc, where we need 
> to restore the toc pointer after the plt stubs returns to the original call 
> site.

Excellent catch. I will update the patch to handle the ppc special case.

Thanks,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r322769 - [RISCV] Propagate -mabi and -march values to GNU assembler.

[Rafael Avila de Espindola via cfe-commits]

With this I am getting a test failure on linux:

 TEST 'Clang :: Driver/riscv-gnutools.c' FAILED 

Script:
--
/home/admin/llvm/build/bin/clang -target riscv32-linux-unknown-elf 
-fno-integrated-as  
--gcc-toolchain=/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk
  
--sysroot=/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot
 /home/admin/llvm/llvm-project/clang/test/Driver/riscv-gnutools.c -###  2>&1 | 
/home/admin/llvm/build/bin/FileCheck -check-prefix=MABI-ILP32 
/home/admin/llvm/llvm-project/clang/test/Driver/riscv-gnutools.c
/home/admin/llvm/build/bin/clang -target riscv32-linux-unknown-elf 
-fno-integrated-as  -march=rv32g 
--gcc-toolchain=/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk
  
--sysroot=/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot
 /home/admin/llvm/llvm-project/clang/test/Driver/riscv-gnutools.c -###  2>&1 | 
/home/admin/llvm/build/bin/FileCheck -check-prefix=MABI-ILP32-MARCH-G 
/home/admin/llvm/llvm-project/clang/test/Driver/riscv-gnutools.c
--
Exit Code: 1

Command Output (stderr):
--
/home/admin/llvm/llvm-project/clang/test/Driver/riscv-gnutools.c:12:16: error: 
expected string not found in input
// MABI-ILP32: 
"{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|}}as"
 "-mabi" "ilp32"
   ^
:1:1: note: scanning from here
clang version 7.0.0
^
:7:42: note: possible intended match here
 
"/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin/ld"
 
"--sysroot=/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot"
 "--hash-style=both" "--eh-frame-hdr" "-m" "elf32lriscv" "-dynamic-linker" 
"/lib/ld-linux-riscv32-ilp32.so.1" "-o" "a.out" "crt1.o" "crti.o" 
"/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32/crtbegin.o"
 
"-L/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/lib32/ilp32"
 
"-L/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib/../lib32"
 
"-L/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib32/ilp32"
 
"-L/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/usr/lib32/ilp32"
 
"-L/home/admin/llvm/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot/lib"
 "/tmp/lit_tmp_9u9TOy/riscv-gnutools-ebce8c.o" "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed" "-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"crtend.o" "crtn.o"
 ^

--

Cheers,
Rafael




Ana Pazos via cfe-commits  writes:

> Author: apazos
> Date: Wed Jan 17 14:09:58 2018
> New Revision: 322769
>
> URL: http://llvm.org/viewvc/llvm-project?rev=322769=rev
> Log:
> [RISCV] Propagate -mabi and -march values to GNU assembler.
>
> When using -fno-integrated-as flag, the gnu assembler produces code
> with some default march/mabi which later causes linker failure due
> to incompatible mabi/march.
>
> In this patch we explicitly propagate -mabi and -march flags to the
> GNU assembler.
>
> In this patch we explicitly propagate -mabi and -march flags to the GNU 
> assembler.
>
> Differential Revision: https://reviews.llvm.org/D41271
>
> Added:
> 
> cfe/trunk/test/Driver/Inputs/multilib_riscv_linux_sdk/riscv64-unknown-linux-gnu/bin/as
> cfe/trunk/test/Driver/riscv-gnutools.c
> Modified:
> cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
>
> Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=322769=322768=322769=diff
> ==
> --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
> +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Wed Jan 17 14:09:58 2018
> @@ -629,6 +629,18 @@ void tools::gnutools::Assembler::Constru
>ppc::getPPCAsmModeForCPU(getCPUName(Args, 
> getToolChain().getTriple(;
>  break;
>}
> +  case llvm::Triple::riscv32:
> +  case llvm::Triple::riscv64: {
> +StringRef ABIName = riscv::getRISCVABI(Args, getToolChain().getTriple());
> +CmdArgs.push_back("-mabi");
> +CmdArgs.push_back(ABIName.data());
> +if (const Arg *A = Args.getLastArg(options::OPT_march_EQ)) {
> +  StringRef MArch = A->getValue();
> +  CmdArgs.push_back("-march");
> +  CmdArgs.push_back(MArch.data());
> +}
> +break;
> +  }
>case llvm::Triple::sparc:
>case llvm::Triple::sparcel: {
>  CmdArgs.push_back("-32");
>
> Added: 
> 

Re: [PATCH] D41318: Start setting dso_local in clang

[Rafael Avila de Espindola via cfe-commits]

Ping.

Is this direction OK? Should a put the time to update the existing tests
to account for dso_local?

I do volunteer to implement the rest of ELF, COFF and MachO once this is
in.

Cheers,
Rafael

Rafael Avila de Espindola  writes:

> Reid Kleckner via Phabricator  writes:
>
>> rnk added inline comments.
>>
>>
>> 
>> Comment at: lib/CodeGen/CodeGenModule.cpp:690-692
>> +  // Only handle ELF for now.
>> +  if (!CGM.getTriple().isOSBinFormatELF())
>> +return false;
>> 
>> Handling COFF here is probably trivial. Everything is dso_local unless it's 
>> dllimport. Does that matter, or is dso_local intended to be an ELF-specific 
>> annotation?
>
> The dso_local attribute is for all file formats. This patch is ELF only
> in the interest of being incremental.
>
> In addition of what this patch shows there will be a lot of test updates
> because of dso_local showing up, so splitting up the patches is probably
> a good thing.
>
> Cheers,
> Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D41318: Start setting dso_local in clang

[Rafael Avila de Espindola via cfe-commits]

Reid Kleckner via Phabricator  writes:

> rnk added inline comments.
>
>
> 
> Comment at: lib/CodeGen/CodeGenModule.cpp:690-692
> +  // Only handle ELF for now.
> +  if (!CGM.getTriple().isOSBinFormatELF())
> +return false;
> 
> Handling COFF here is probably trivial. Everything is dso_local unless it's 
> dllimport. Does that matter, or is dso_local intended to be an ELF-specific 
> annotation?

The dso_local attribute is for all file formats. This patch is ELF only
in the interest of being incremental.

In addition of what this patch shows there will be a lot of test updates
because of dso_local showing up, so splitting up the patches is probably
a good thing.

Cheers,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39831: [Driver] Make the use of relax relocations a per target option

[Rafael Avila de Espindola via cfe-commits]

Petr Hosek  writes:

> That's the Fuchsia CMake cache file which is used to build Fuchsia
> toolchain, it's not needed there anymore because Fuchsia Clang driver now
> handles this. I haven't touched Clang's CMakeLists.txt which defines
> the ENABLE_X86_RELAX_RELOCATIONS option.

Oops, I guess I should have read the filename.

LGTM.

Thanks,
Rafael
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39831: [Driver] Make the use of relax relocations a per target option

[Rafael Avila de Espindola via cfe-commits]

I am probably missing something, but the patch has

-# This is a "Does your linker support it?" option that only applies
-# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
-# For x86-64 Linux, it's supported by LLD and by GNU linkers since
-# binutils 2.27, so one can hope that all Linux hosts in use handle it.
-# Ideally this would be settable as a per-target option.
-set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")

And I don't see what that is replaced with.

Cheers,
Rafael

Petr Hosek  writes:

> I kept the CMake option, so by default the driver will use the value set
> through CMake as before but individual targets can now set their platform
> default.
>
> On Tue, Nov 21, 2017 at 1:30 PM Rafael Avila de Espindola <
> rafael.espind...@gmail.com> wrote:
>
>> Petr Hosek via Phabricator  writes:
>>
>>
>> > -# This is a "Does your linker support it?" option that only applies
>> > -# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
>> > -# For x86-64 Linux, it's supported by LLD and by GNU linkers since
>> > -# binutils 2.27, so one can hope that all Linux hosts in use handle it.
>> > -# Ideally this would be settable as a per-target option.
>> > -set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
>> > -
>>
>> We still have to be able to set it via cmake.
>>
>> Cheers,
>> Rafeal
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D39831: [Driver] Make the use of relax relocations a per target option

[Rafael Avila de Espindola via cfe-commits]

Petr Hosek via Phabricator  writes:


> -# This is a "Does your linker support it?" option that only applies
> -# to x86-64 ELF targets.  All Fuchsia target linkers do support it.
> -# For x86-64 Linux, it's supported by LLD and by GNU linkers since
> -# binutils 2.27, so one can hope that all Linux hosts in use handle it.
> -# Ideally this would be settable as a per-target option.
> -set(ENABLE_X86_RELAX_RELOCATIONS ON CACHE BOOL "")
> -

We still have to be able to set it via cmake.

Cheers,
Rafeal
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [cfe-commits] r124613 - /cfe/trunk/lib/Frontend/FrontendActions.cpp

[Rafael Avila de Espindola via cfe-commits]

Daniel Dunbar  writes:

> Author: ddunbar
> Date: Mon Jan 31 16:00:44 2011
> New Revision: 124613
>
> URL: http://llvm.org/viewvc/llvm-project?rev=124613=rev
> Log:
> libclang: Don't allow RemoveFileOnSignal to be called via libclang, badness 
> can
> ensue.


Sorry for digging out such an old commit, but do you remember what
badness that ensues if RemoveFileOnSignal is called?

Would the same badness ensue if FILE_FLAG_DELETE_ON_CLOSE were used on
Windows?


Thanks,
Rafael


> Modified:
> cfe/trunk/lib/Frontend/FrontendActions.cpp
>
> Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=124613=124612=124613=diff
> ==
> --- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
> +++ cfe/trunk/lib/Frontend/FrontendActions.cpp Mon Jan 31 16:00:44 2011
> @@ -104,7 +104,10 @@
>  return true;
>}
>  
> -  OS = CI.createDefaultOutputFile(true, InFile);
> +  // We use createOutputFile here because this is exposed via libclang, and 
> we
> +  // must disable the RemoveFileOnSignal behavior.
> +  OS = CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true,
> +   /*RemoveFileOnSignal=*/false, InFile);
>if (!OS)
>  return true;
>  
>
>
> ___
> cfe-commits mailing list
> cfe-comm...@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D28807: [Clang] - Update code to match upcoming llvm::zlib API.

[Rafael Avila de Espindola via cfe-commits]

LGTM

George Rimar via Phabricator  writes:

> grimar created this revision.
>
> https://reviews.llvm.org/D28684 changed llvm::zlib to return Error instead of 
> Status.
> It was accepted and committed in r292214, but then reverted in r292217
> because I missed that clang code also needs to be updated.
>
> Patch do that.
>
>
> https://reviews.llvm.org/D28807
>
> Files:
>   lib/Serialization/ASTReader.cpp
>   lib/Serialization/ASTWriter.cpp
>
>
> Index: lib/Serialization/ASTWriter.cpp
> ===
> --- lib/Serialization/ASTWriter.cpp
> +++ lib/Serialization/ASTWriter.cpp
> @@ -73,6 +73,7 @@
>  #include "llvm/Support/Casting.h"
>  #include "llvm/Support/Compression.h"
>  #include "llvm/Support/EndianStream.h"
> +#include "llvm/Support/Error.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/MemoryBuffer.h"
>  #include "llvm/Support/OnDiskHashTable.h"
> @@ -1986,6 +1987,30 @@
>  free(const_cast(SavedStrings[I]));
>  }
>  
> +static void emitBlob(llvm::BitstreamWriter , StringRef Blob,
> + unsigned SLocBufferBlobCompressedAbbrv,
> + unsigned SLocBufferBlobAbbrv) {
> +  typedef ASTWriter::RecordData::value_type RecordDataType;
> +
> +  // Compress the buffer if possible. We expect that almost all PCM
> +  // consumers will not want its contents.
> +  SmallString<0> CompressedBuffer;
> +  if (llvm::zlib::isAvailable()) {
> +llvm::Error E = llvm::zlib::compress(Blob.drop_back(1), 
> CompressedBuffer);
> +if (!E) {
> +  RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB_COMPRESSED,
> + Blob.size() - 1};
> +  Stream.EmitRecordWithBlob(SLocBufferBlobCompressedAbbrv, Record,
> +CompressedBuffer);
> +  return;
> +}
> +llvm::consumeError(std::move(E));
> +  }
> +
> +  RecordDataType Record[] = {SM_SLOC_BUFFER_BLOB};
> +  Stream.EmitRecordWithBlob(SLocBufferBlobAbbrv, Record, Blob);
> +}
> +
>  /// \brief Writes the block containing the serialized form of the
>  /// source manager.
>  ///
> @@ -2094,20 +2119,8 @@
>  const llvm::MemoryBuffer *Buffer =
>  Content->getBuffer(PP.getDiagnostics(), PP.getSourceManager());
>  StringRef Blob(Buffer->getBufferStart(), Buffer->getBufferSize() + 
> 1);
> -
> -// Compress the buffer if possible. We expect that almost all PCM
> -// consumers will not want its contents.
> -SmallString<0> CompressedBuffer;
> -if (llvm::zlib::compress(Blob.drop_back(1), CompressedBuffer) ==
> -llvm::zlib::StatusOK) {
> -  RecordData::value_type Record[] = {SM_SLOC_BUFFER_BLOB_COMPRESSED,
> - Blob.size() - 1};
> -  Stream.EmitRecordWithBlob(SLocBufferBlobCompressedAbbrv, Record,
> -CompressedBuffer);
> -} else {
> -  RecordData::value_type Record[] = {SM_SLOC_BUFFER_BLOB};
> -  Stream.EmitRecordWithBlob(SLocBufferBlobAbbrv, Record, Blob);
> -}
> +emitBlob(Stream, Blob, SLocBufferBlobCompressedAbbrv,
> + SLocBufferBlobAbbrv);
>}
>  } else {
>// The source location entry is a macro expansion.
> Index: lib/Serialization/ASTReader.cpp
> ===
> --- lib/Serialization/ASTReader.cpp
> +++ lib/Serialization/ASTReader.cpp
> @@ -72,6 +72,7 @@
>  #include "llvm/Bitcode/BitstreamReader.h"
>  #include "llvm/Support/Compression.h"
>  #include "llvm/Support/Compiler.h"
> +#include "llvm/Support/Error.h"
>  #include "llvm/Support/ErrorHandling.h"
>  #include "llvm/Support/FileSystem.h"
>  #include "llvm/Support/MemoryBuffer.h"
> @@ -1278,10 +1279,15 @@
>  unsigned RecCode = SLocEntryCursor.readRecord(Code, Record, );
>  
>  if (RecCode == SM_SLOC_BUFFER_BLOB_COMPRESSED) {
> +  if (!llvm::zlib::isAvailable()) {
> +Error("zlib is not available");
> +return nullptr;
> +  }
>SmallString<0> Uncompressed;
> -  if (llvm::zlib::uncompress(Blob, Uncompressed, Record[0]) !=
> -  llvm::zlib::StatusOK) {
> -Error("could not decompress embedded file contents");
> +  if (llvm::Error E =
> +  llvm::zlib::uncompress(Blob, Uncompressed, Record[0])) {
> +Error("could not decompress embedded file contents: " +
> +  llvm::toString(std::move(E)));
>  return nullptr;
>}
>return llvm::MemoryBuffer::getMemBufferCopy(Uncompressed, Name);
>
>
> Index: lib/Serialization/ASTWriter.cpp
> ===
> --- lib/Serialization/ASTWriter.cpp
> +++ lib/Serialization/ASTWriter.cpp
> @@ -73,6 +73,7 @@
>  #include "llvm/Support/Casting.h"
>  #include "llvm/Support/Compression.h"
>  #include "llvm/Support/EndianStream.h"
> +#include