[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 230494.
serge-sans-paille added a comment.

Added extra test for arch support (warns if the flag is used on unsupported 
targets, and checks it's effectively unused), cc @sylvestre.ledru


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur to make your reviewer job easier, I've setup validation for that 
patch, see https://github.com/serge-sans-paille/llvm-project/pull/2/checks
It build and validates fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D61446#1681841 , @Meinersbur wrote:

> Keep in mind that for static linking you will need something that pulls-in a 
> symbol from the pass static library. In this patch, `NewPMDriver.cpp` does it 
> for `opt` by calling `get##Ext##PluginInfo()`. In clang, it is 
> `BackendUtil.cpp`. But nothing for `bugpoint` hence it doesn't contain either 
> Polly not the Goodbye pass (However, llvm-reduce is in the works, we might 
> consider bugpoint deprecated).


Done, thanks.

> Could you add documentation for how to write a tool that uses loadable 
> plugins to document the way it should be done?

It's natural if  we use the NewPM, so I've added some doc in that direction.




Comment at: polly/lib/Support/RegisterPasses.cpp:726
+
+#ifndef LLVM_POLLY_LINK_INTO_TOOLS
+extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo

Meinersbur wrote:
> Meinersbur wrote:
> > [serious] `LLVM_POLLY_LINK_INTO_TOOLS` is a cmake configuration parameter, 
> > but not a preprocessor symbol. Hence, `LLVM_POLLY_LINK_INTO_TOOLS` is never 
> > defined.
> > 
> > Error on Windows:
> > ```
> > Bye.lib(Bye.cpp.obj) : error LNK2005: llvmGetPassPluginInfo already defined 
> > in Polly.lib(RegisterPasses.cpp.obj)
> > bin\opt.exe : fatal error LNK1169: one or more multiply defined symbols 
> > found
> > ```
> Before you try to fix this preprocessor symbol, consider that Polly compiles 
> a loadable module (to be used with `-load`) __and__ a library (static or 
> dynamic depending on `BUILD_SHARED_LIBS`) __independent__ of 
> `LLVM_POLLY_LINK_INTO_TOOLS`. That is, the loadable module must contain 
> `llvmGetPassPluginInfo`, but not the library. That is, a preprocessor symbol 
> that is the same for both will not work.
> 
> PLEASE, just keep the Polly.cpp (and move `llvmGetPassPluginInfo` in there; 
> the static initializer indeed has to stay here as it will be used by both), 
> it will make things easier as it already has been shown to work already. Do 
> the same for Bye. 
> 
> If you really insist on removing the `Polly.cpp`, do so in a follow-up patch. 
> In that case you will have to rework the CMakeLists.txt to only build one, 
> either the loadable module or the library.
In `add_llvm_pass_plugin` there's 

```
  if(LLVM_${name_upper}_LINK_INTO_TOOLS)
  target_compile_definitions(${name} PRIVATE 
LLVM_${name_upper}_LINK_INTO_TOOLS)
  set_property(TARGET ${name} APPEND PROPERTY COMPILE_DEFINITIONS 
LLVM_LINK_INTO_TOOLS)
  if (TARGET intrinsics_gen)
add_dependencies(obj.${name} intrinsics_gen)
  endif()
  endif()
```

So the macro definition should be there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 230415.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

Take into account most of the reviews.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Plugin/Polly.cpp
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rnk : up ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 229127.
serge-sans-paille added a comment.

First rework pass, sevral comments have been taken into account but not all of 
them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Plugin/Polly.cpp
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@sylvestre.ledru did the testing and benchmarking on firefox (see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1588710#c12), everything seems ok, 
let's move forward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@sylvestre.ledru : quick backport for llvm9: 
https://sergesanspaille.fedorapeople.org/0001-Stack-clash-mir-attempt.patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rnk what's your take on that version?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 227194.
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.

Moved to a simpler, easier to review, less error prone implementation. 
Validates just fines on llvm bootstrap an cpython code base. @sylvestre.ledru 
that's the patch you can play with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 226109.
serge-sans-paille added a comment.

Temporarily comment out call support as free probe, everything else passes 
validation but a call may have some stack effect I don't handle (yet).


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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Some benchmark / instrumentation report: due to the way memory moves are 
ordered in the entry block, there tend to be relatively few free probes between 
two stack growth within a function, and a large number after the last stack 
growth.

When recompiling llc, I only get 18 free probes between stack growth, and a 
large amount (several thousands) after the last stack growth.
When recompiling cpython, I only get 4 free probes between stack growth.

This may be improved by rescheduling instructions / changing stack object 
layout but that's probably not worth the effort (there's not that much 
functions with stack space > a page)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 226043.
serge-sans-paille added a comment.

Update documentation as suggested by @sylvestre.ledru 
Corner case when a write was touching memory beyond the allocated stack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 226002.
serge-sans-paille added a comment.

Better integration with MachineInstr description
Handle stacks with multiple stack objects
More test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes-mutliple-objects.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll
  llvm/test/CodeGen/X86/stack-clash-unknown-call.ll

Index: llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-unknown-call.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg);
+
+define void @foo() local_unnamed_addr #0 {
+
+;CHECK-LABEL: foo:
+;CHECK: # %bb.0:
+;CHECK-NEXT:	subq	$4096, %rsp # imm = 0x1000
+; it's important that we don't use the call as a probe here
+;CHECK-NEXT:	movq	$0, (%rsp)
+;CHECK-NEXT:	subq	$3912, %rsp # imm = 0xF48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8016
+;CHECK-NEXT:	movq	%rsp, %rdi
+;CHECK-NEXT:	movl	$8000, %edx # imm = 0x1F40
+;CHECK-NEXT:	xorl	%esi, %esi
+;CHECK-NEXT:	callq	memset
+;CHECK-NEXT:	addq	$8008, %rsp # imm = 0x1F48
+;CHECK-NEXT:	.cfi_def_cfa_offset 8
+;CHECK-NEXT:	retq
+
+  %a = alloca i8, i64 8000, align 16
+  call void @llvm.memset.p0i8.i64(i8* align 16 %a, i8 0, i64 8000, i1 false)
+  ret void
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,27 @@

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 225580.
serge-sans-paille added a comment.

Explicilty prefer existing mechanism for windows.
Split loop/block allocation strategy to different functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, 880(%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK: # %bb.0:
+; CHECK-NEXT: 	 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Extra note: an older version of the patch has been tested by the firefox team 
without much performance impact, (and no test failure), see 
https://bugzilla.mozilla.org/show_bug.cgi?id=1588710


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 225462.
serge-sans-paille added a comment.

Moved the implementation to a specialization of `emitStackProbeInline` that 
used to be Windows-centric. Provide a generic implementation that generates 
inline assembly instead. that way we don't clutter existing functions, and only 
add a new case to `emitSPUpdate`.

Handle large stack allocation with a loop instead of a (large) basic block.

@rnk: this is far less intrusive and more integrated to the existing structure, 
thanks a lot for hinting in that direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-large.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, 880(%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 225049.
serge-sans-paille added a comment.

Get rid of static mapping + update test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 288
+; CHECK-NEXT:  movl	$1, 264(%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$280, %rsp # imm = 0x118
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,27 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, -128(%rsp,%rdi,4)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,28 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:  subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT:  .cfi_def_cfa_offset 7888
+; CHECK-NEXT:  movl	$1, 880(%rsp)
+; CHECK-NEXT:  subq	$3784, %rsp # imm = 0xEC8
+; CHECK-NEXT:  movq	$0, (%rsp)
+; CHECK-NEXT:  movl	-128(%rsp), %eax
+; CHECK-NEXT:  addq	$7880, %rsp # imm = 0x1EC8
+; CHECK-NEXT:  .cfi_def_cfa_offset 8
+; CHECK-NEXT:  retq
+
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,31 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo() local_unnamed_addr #0 {
+
+; CHECK-LABEL: foo:
+; CHECK: # %bb.0:
+; CHECK-NEXT: 	 subq	$4096, %rsp # imm = 0x1000
+; CHECK-NEXT: 	 .cfi_def_cfa_offset 7888
+; CHECK-NEXT: 	 movl	$1, 264(%rsp)

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Another test run on an x264 encoder (source: 
https://openbenchmarking.org/test/pts/x264)

Compiled with -O2 and with or without -fstack-clash-protection; Run without 
threads (`x265 --pools 1 -F 1 ./Bosphorus_1920x1080_120fps_420_8bit_YUV.y4m 
/dev/null`)

**Clang**

  baseline:  318.60s 
  protection:  317.72s

So no performance impact beyond noise.

The compilation inserts 44 inline probes in 9 functions.

**gcc**

Out of comparison, with gcc 8.2 (yeah, it's a bit old), I get (same flags & 
setup)

  baseline : 417.53 sec
  protected : 412.6 sec 

The compilations inserts inline probes in 22 functions.
So gcc inserts more probes, the impact on performance is equally surprising. I 
need to gather more points to get some statistical informations there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

> For maintenance reasons, I'd really prefer it if we could find a way to reuse 
> the existing code that calls an external stack probe function. What do you 
> think about taking a look at X86RetpolineThunks.cpp and doing something 
> similar to that? Basically, when the user sets -fstack-clash-protection, LLVM 
> will emit a small comdat+weak function into every object file that has the 
> same ABI as the existing stack probe mechanism. For other prior art, you can 
> also look at how __clang_call_terminate works.

@rnk I 100% understand the reasoning, and will have a look to the files you're 
pointing at. AFAIU the approach you're suggesting is incompatible with the « 
free probe » algorithm that moves instruction in between stack allocations, but 
let me confirm that first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Some benchmarks, running the python performance suite from recompiled cpython 
build, one built with `-O2` (baseline) and one built with `-O2 
-fstack-clash-protection` (protection). Surprisingly enough, the stack 
protection makes the code faster in various scenario, but none of these changes 
present significant regression, according to me.

  baseline.json
  =
  
  Performance version: 0.9.1
  Report on Linux-3.10.0-891.el7.x86_64-x86_64-with-redhat-7.5-Maipo
  Number of logical CPUs: 8
  Start date: 2019-10-15 11:16:28.425344
  End date: 2019-10-15 11:37:55.064599
  
  protection.json
  ===
  
  Performance version: 0.9.1
  Report on Linux-3.10.0-891.el7.x86_64-x86_64-with-redhat-7.5-Maipo
  Number of logical CPUs: 8
  Start date: 2019-10-15 10:55:24.270166
  End date: 2019-10-15 11:16:27.195366
  
  
+-+---+-+--+---+
  | Benchmark   | baseline.json | protection.json | Change   | 
Significance  |
  
+=+===+=+==+===+
  | 2to3| 435 ms| 448 ms  | 1.03x slower | 
Significant (t=-4.26) |
  
+-+---+-+--+---+
  | chameleon   | 15.0 ms   | 14.7 ms | 1.02x faster | 
Significant (t=4.76)  |
  
+-+---+-+--+---+
  | chaos   | 176 ms| 174 ms  | 1.02x faster | 
Not significant   |
  
+-+---+-+--+---+
  | crypto_pyaes| 153 ms| 150 ms  | 1.02x faster | 
Not significant   |
  
+-+---+-+--+---+
  | deltablue   | 11.9 ms   | 11.9 ms | 1.00x slower | 
Not significant   |
  
+-+---+-+--+---+
  | django_template | 210 ms| 223 ms  | 1.06x slower | 
Significant (t=-3.84) |
  
+-+---+-+--+---+
  | dulwich_log | 96.1 ms   | 102 ms  | 1.06x slower | 
Significant (t=-8.42) |
  
+-+---+-+--+---+
  | fannkuch| 703 ms| 698 ms  | 1.01x faster | 
Not significant   |
  
+-+---+-+--+---+
  | float   | 161 ms| 160 ms  | 1.01x faster | 
Not significant   |
  
+-+---+-+--+---+
  | genshi_text | 45.5 ms   | 45.5 ms | 1.00x faster | 
Not significant   |
  
+-+---+-+--+---+
  | genshi_xml  | 95.3 ms   | 95.2 ms | 1.00x faster | 
Not significant   |
  
+-+---+-+--+---+
  | go  | 392 ms| 382 ms  | 1.03x faster | 
Significant (t=5.95)  |
  
+-+---+-+--+---+
  | hexiom  | 16.0 ms   | 15.9 ms | 1.01x faster | 
Not significant   |
  
+-+---+-+--+---+
  | html5lib| 137 ms| 135 ms  | 1.01x faster | 
Not significant   |
  
+-+---+-+--+---+
  | json_dumps  | 18.8 ms   | 17.9 ms | 1.05x faster | 
Significant (t=4.81)  |
  
+-+---+-+--+---+
  | json_loads  | 41.3 us   | 40.2 us | 1.03x faster | 
Significant (t=2.53)  |
  
+-+---+-+--+---+
  | logging_format  | 17.4 us   | 17.5 us | 1.00x slower | 
Not significant   |
  
+-+---+-+--+---+
  | logging_silent  | 509 ns| 500 ns  | 1.02x faster | 
Not significant   |
  
+-+---+-+--+---+
  | logging_simple  | 14.3 us   | 14.8 us | 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 224606.
serge-sans-paille added a comment.

Ensure the distance between two probes is at max PAGE_SIZE.
Use Calls as free probes.
Fix alignment for dynamic alloca

This passes the llvm-test suite, and thanks to the use of calls, no inserted 
probe are needed to compile sqlite!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,20 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK: subq   $4096, %rsp
+; the store location is not statically known, no free probe
+; CHECK: movq   $0, (%rsp)
+; CHECK: subq   ${{[0-9]+}}, %rsp
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,21 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Some early stats: on the sqlite amalgamation [0], the free probe reuse  allows 
to skip 123 out of the 474 probes needed during frame lowering.

[0] https://www.sqlite.org/download.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 224431.
serge-sans-paille edited the summary of this revision.
serge-sans-paille added a comment.

Added test case, statistics and refactor interactions with existing stack 
probing mechanism.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/Target/X86/X86CallFrameOptimization.cpp
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-no-free-probe.ll
@@ -0,0 +1,20 @@
+; RUN: llc < %s | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i64 %i) local_unnamed_addr #0 {
+; CHECK: subq   $4096, %rsp
+; the store location is not statically known, no free probe
+; CHECK: movq   $0, (%rsp)
+; CHECK: subq   ${{[0-9]+}}, %rsp
+  %a = alloca i32, i32 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 %i
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
+
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,21 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s | FileCheck %s
+

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 3 inline comments as done.
serge-sans-paille added inline comments.



Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:400
+!(STI.isOSWindows() && !STI.isTargetMachO());
+if (InlineStackClashProtector && !InEpilogue) {
+  const uint64_t PageSize = TLI.getStackProbeSize(MF);

efriedma wrote:
> Why is this code in a different location from the stack probing code that 
> generates a call?
Because `BuildStackAdjustment` has more callsites than just emitPrologue and we 
want to capture all stack manipulation.



Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:408
+CurrentAbsOffset += ChunkSize;
+MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
+

efriedma wrote:
> This algorithm needs some documentation; it isn't at all obvious what it's 
> doing.  Particularly the interaction with "free" stack probes.
> 
> Should we generate a loop if the stack frame is large?
> Should we generate a loop if the stack frame is large?

That's an option. it's more complex to make looping compatible with free 
probes, but that's possible.



Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:408
+CurrentAbsOffset += ChunkSize;
+MI->getOperand(3).setIsDead(); // The EFLAGS implicit def is dead.
+

serge-sans-paille wrote:
> efriedma wrote:
> > This algorithm needs some documentation; it isn't at all obvious what it's 
> > doing.  Particularly the interaction with "free" stack probes.
> > 
> > Should we generate a loop if the stack frame is large?
> > Should we generate a loop if the stack frame is large?
> 
> That's an option. it's more complex to make looping compatible with free 
> probes, but that's possible.
> This algorithm needs some documentation
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a subscriber: eli.friedman.
serge-sans-paille added a comment.

@efriedma  the free probe algorithm requires more testing, and I'd like to take 
into account memset and memcpy as free probes too. To showcase this algorithm, 
consider the following LLVM bitcode:

  define i32 @foo() local_unnamed_addr {
%a = alloca i32, i64 2000, align 16
%b = getelementptr inbounds i32, i32* %a, i64 1198
store volatile i32 1, i32* %b
%c = load volatile i32, i32* %a
ret i32 %c
  }

when compiled with `llc` it outputs the following assembly:

  foo:# @foo
  subq$7880, %rsp # imm = 0x1EC8
  movl$1, 4664(%rsp)
  movl-128(%rsp), %eax
  addq$7880, %rsp # imm = 0x1EC8
  retq

When `probe-stack` is set to `inline-asm` it outputs

  foo:# @foo
subq$4096, %rsp # imm = 0x1000
movl$1, 880(%rsp)
subq$3784, %rsp # imm = 0xEC8
movq$0, (%rsp)
movl-128(%rsp), %eax
addq$7880, %rsp # imm = 0x1EC8
retq

The stack allocation is split in two, but only one MOV is added, the first one 
is what I call a free probe. Turns out we could only use natural probes here, I 
need to implement that :-)

As a comparison, setting `probe-stack` to a random function name like 
`__probe_stack` outputs the following:

  foo:# @foo
movl$8008, %eax # imm = 0x1F48
callq   __probe_stack
subq%rax, %rsp
movl$1, 4792(%rsp)
movl(%rsp), %eax
addq$8008, %rsp # imm = 0x1F48
retq

which requires runtime support (to provide `__stack_probe`), and a function 
call overhead, while ideally just an extra `sub %rsp` would be needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 224254.
serge-sans-paille added a comment.

Added documentation + release note entry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,21 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i32 %n) local_unnamed_addr #0 {
+; CHECK: subq   $4096, %rsp
+; CHECK: movq   $0, (%rsp)
+  %a = alloca i32, i32 %n, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -121,6 +121,8 @@
 
 def SDT_X86SEG_ALLOCA : SDTypeProfile<1, 1, [SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
 
+def SDT_X86PROBED_ALLOCA : SDTypeProfile<1, 1, [SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
+
 def SDT_X86EHRET : SDTypeProfile<0, 1, [SDTCisInt<0>]>;
 
 def SDT_X86TCRET : SDTypeProfile<0, 2, [SDTCisPtrTy<0>, SDTCisVT<1, i32>]>;
@@ -292,6 +294,9 @@
 def X86SegAlloca : SDNode<"X86ISD::SEG_ALLOCA", 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@efriedma alos compared to `probe-stack` with a function, this version has the 
ability to use existing MOV operations to avoid generating probes, which looks 
like a big plus to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 224196.
serge-sans-paille added a comment.

Move to `stack-probe` compatibility, using a dedicated name to trigger inline 
assembly. It looks better to me because

1. it leverage existing mechanics
2. it has a finer grain


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,19 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,21 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,25 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr #0 {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
@@ -0,0 +1,18 @@
+; RUN: llc < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i32 %n) local_unnamed_addr #0 {
+; CHECK: subq   $4096, %rsp
+; CHECK: movq   $0, (%rsp)
+  %a = alloca i32, i32 %n, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
+
+attributes #0 =  {"probe-stack"="inline-asm"}
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -121,6 +121,8 @@
 
 def SDT_X86SEG_ALLOCA : SDTypeProfile<1, 1, [SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
 
+def SDT_X86PROBED_ALLOCA : SDTypeProfile<1, 1, [SDTCisVT<0, iPTR>, SDTCisVT<1, iPTR>]>;
+
 def SDT_X86EHRET : SDTypeProfile<0, 1, [SDTCisInt<0>]>;
 
 def SDT_X86TCRET : SDTypeProfile<0, 2, [SDTCisPtrTy<0>, SDTCisVT<1, 

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@efriedma : there's indeed an intersection with the `probe-stack` attribute. 
The `probe-stack` attribute (a) forces a function call, and (b) this function 
call only happens **before** the stack gets expanded.

(a) is probably a performance issue in several cases, plus it requires an extra 
register (that's mentioned in  https://reviews.llvm.org/D9653) 
(b) is an issue, as pointed out in https://lwn.net/Articles/726587/ (grep for 
valgrind)  : from valgrind point of view, accessing un-allocated stack memory 
triggers error, and we probably want to please valgrind

Doing the call *after* the stack allocation is also not an option, as a signal 
could be raised between the stack allocation and the stack probing, escaping 
the stack probe if a custom signal handler is executed.

That being said, I do think it would be a good thing to have a special value 
for `probe-stack`, say `probe-stack=inline-asm`, that would trigger generation 
of inlined assembly as I do. That way we have all the pieces in one place, with 
different strategies. And we would have clang set the attribute for each 
function when `-fstack-clash-protection` is given.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya, dschuff.
Herald added projects: clang, LLVM.

Implement protection against the stack clash attack [0].

Probe stack allocation every PAGE_SIZE during frame lowering or dynamic 
allocation to make sure the  page guard, if any, is touched when touching the 
stack, in a similar manner to GCC[1].

If possible, use MOV already present in the entry block instead of generating 
new ones.

Only implemented for x86.

[0] https://www.qualys.com/2017/06/19/stack-clash/stack-clash.txt
[1] https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00556.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68720

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/stack-clash-protection.c
  clang/test/Driver/stack-clash-protection.c
  llvm/include/llvm/CodeGen/CommandFlags.inc
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/Target/X86/X86FrameLowering.cpp
  llvm/lib/Target/X86/X86FrameLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
  llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
  llvm/test/CodeGen/X86/stack-clash-medium.ll
  llvm/test/CodeGen/X86/stack-clash-small.ll

Index: llvm/test/CodeGen/X86/stack-clash-small.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-small.ll
@@ -0,0 +1,17 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; only one stack growth
+; CHECK: subq
+; CHECK-NOT: subq
+  %a = alloca i32, i64 100, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 98
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-medium.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium.ll
@@ -0,0 +1,19 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; two stack growth, with a natural probe inbetween and an extra probe afterward
+; CHECK: subq
+; CHECK: movl
+; CHECK: subq
+; CHECK: movq $0
+  %a = alloca i32, i64 2000, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-medium-natural-probes.ll
@@ -0,0 +1,23 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo() local_unnamed_addr {
+; two stack growth, with an extra probe inbetween and a natural probe afterward
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+; CHECK: subq
+; CHECK-NOT: movq $0
+; CHECK: mov
+  %a = alloca i32, i64 2000, align 16
+  %b0 = getelementptr inbounds i32, i32* %a, i64 98
+  %b1 = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b0
+  store volatile i32 1, i32* %b1
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll
@@ -0,0 +1,16 @@
+; RUN: llc --stack-clash-protector < %s | FileCheck %s
+
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; CHECK-LABEL: foo:
+define i32 @foo(i32 %n) local_unnamed_addr {
+; CHECK: subq   $4096, %rsp
+; CHECK: movq   $0, (%rsp)
+  %a = alloca i32, i32 %n, align 16
+  %b = getelementptr inbounds i32, i32* %a, i64 1198
+  store volatile i32 1, i32* %b
+  %c = load volatile i32, i32* %a
+  ret i32 %c
+}
Index: llvm/lib/Target/X86/X86InstrInfo.td
===
--- llvm/lib/Target/X86/X86InstrInfo.td
+++ llvm/lib/Target/X86/X86InstrInfo.td
@@ -121,6 +121,8 @@
 
 def SDT_X86SEG_ALLOCA : 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 221321.
serge-sans-paille added a comment.

Updates:

- fix typo in documentation
- take into account @andwar advices
- improve shared/static build automation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur any feedback on this update?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Up :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 217604.
serge-sans-paille added a comment.

@Meinersbur patch rebased. I removed the linker trick (which only work for 
global variables, not function, anyway), as it's no longer needed:

  target_compile_definitions(${name} PRIVATE LLVM_${name_upper}_LINK_INTO_TOOLS)

is used to only provide the symbol if we're in shared library mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = 

[PATCH] D56343: [clang-tidy] Refactor: Extract Class CheckRunner on check_clang_tidy.py

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

Sorry for the long delay, this LGTM


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

https://reviews.llvm.org/D56343



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

[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

[serge via Phabricator via cfe-commits]

serge-sans-paille added a reviewer: hfinkel.
serge-sans-paille added a subscriber: hfinkel.
serge-sans-paille added a comment.

Adding @hfinkel to the thread to confirm it's okay to add this behavior, as he 
authored an RFC on the subject (see 
https://lists.llvm.org/pipermail/llvm-dev/2016-November/107625.html)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65616



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D65616: Ignore -fsemantic-interposition/-fno-semantic-interposition flag for gcc compatibility

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65616



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 213176.
serge-sans-paille added a comment.

Also fix symbol redefinition during static builds on Windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/include/llvm/Support/Compiler.h
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur nop, forgot that one, I'll have a look, thanks for pointing that 
out.


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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 212567.
serge-sans-paille added a comment.

Make validation more resilient depending on shared/static build.


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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur I've updated the test case to test extension point if the extension 
is linked in, this is not so intrusive, I'm happy with the solution.

Also fixed some linkage options when using llvm dynlib, validation looks nice 
on my side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 212509.
serge-sans-paille added a comment.
Herald added subscribers: dexonsmith, steven_wu.

- Test extension point registration when extension is built-in
- Correctly handle -DLLVM_LINK_LLVM_DYLIB=0
- Fix a few details


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/test/Feature/load_extension.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/opt-O0-pipeline.ll
  llvm/test/Other/opt-O2-pipeline.ll
  llvm/test/Other/opt-O3-pipeline.ll
  llvm/test/Other/opt-Os-pipeline.ll
  llvm/test/lit.cfg.py
  llvm/test/lit.site.cfg.py.in
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 210273.
serge-sans-paille marked 5 inline comments as done and 2 inline comments as 
done.
serge-sans-paille added a comment.

Added a `Bye` project in examples/
Fixed linking of plugins into core tools
Fixed dependency issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/examples/Bye/Bye.cpp
  llvm/examples/Bye/CMakeLists.txt
  llvm/examples/CMakeLists.txt
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ 

[PATCH] D64129: [clang-scan-view] Force utf-8 when handling report (python2 only)

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: michaelplatings.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If the report file contains unicode characters, under Python2, opening the file 
does not decode it to utf-8 by default.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64129

Files:
  clang/tools/scan-view/share/ScanView.py


Index: clang/tools/scan-view/share/ScanView.py
===
--- clang/tools/scan-view/share/ScanView.py
+++ clang/tools/scan-view/share/ScanView.py
@@ -764,11 +764,11 @@
 variables['report'] = m.group(2)
 
 try:
-f = open(path,'r')
+f = open(path,'rb')
 except IOError:
 return self.send_404()
 fs = os.fstat(f.fileno())
-data = f.read()
+data = f.read().decode('utf-8')
 for a,b in kReportReplacements:
 data = a.sub(b % variables, data)
 return self.send_string(data, ctype, mtime=fs.st_mtime)


Index: clang/tools/scan-view/share/ScanView.py
===
--- clang/tools/scan-view/share/ScanView.py
+++ clang/tools/scan-view/share/ScanView.py
@@ -764,11 +764,11 @@
 variables['report'] = m.group(2)
 
 try:
-f = open(path,'r')
+f = open(path,'rb')
 except IOError:
 return self.send_404()
 fs = os.fstat(f.fileno())
-data = f.read()
+data = f.read().decode('utf-8')
 for a,b in kReportReplacements:
 data = a.sub(b % variables, data)
 return self.send_string(data, ctype, mtime=fs.st_mtime)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 10 inline comments as done.
serge-sans-paille added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:808
 
+if(NOT llvm-pass-plugins)
+# Target used to hold global properties referencable from 
generator-expression

Meinersbur wrote:
> [serious] I get the following error:
> ```
> CMake Error at cmake/modules/AddLLVM.cmake:813 (add_custom_target):
>   add_custom_target cannot create target "llvm-pass-plugins" because another
>   target with the same name already exists.  The existing target is a custom
>   target created in source directory "/home/meinersbur/src/llvm".  See
>   documentation for policy CMP0002 for more details.
> Call Stack (most recent call first):
>   projects/compiler-rt/test/CMakeLists.txt:2 (include)
> ```
> 
> What you meant to use is
> ```
> if (NOT TARGET llvm-pass-plugins)
> ```
> See https://cmake.org/cmake/help/latest/command/if.html
I'm no longer using that mechanism, it was showing some limits wrt. cmake 
generator-expression and export set. 



Comment at: llvm/cmake/modules/AddLLVM.cmake:851
+"extern \"C\" ::llvm::PassPluginLibraryInfo 
LLVM_ATTRIBUTE_WEAK llvmGetPassPluginInfo() { return ${entry_point}(); }")
+  target_sources(${register_llvm_pass_plugin_EXTRA_LOADABLE} PRIVATE 
${CMAKE_CURRENT_BINARY_DIR}/${register_llvm_pass_plugin_EXTRA_LOADABLE}_plugin_entry_point.cpp)
+endif()

Meinersbur wrote:
> [serious] Under Windows, I get the following error:
> ```
> CMake Error at cmake/modules/AddLLVM.cmake:853 (target_sources):
>   target_sources called with non-compilable target type
> Call Stack (most recent call first):
>   tools/polly/lib/CMakeLists.txt:164 (register_llvm_pass_plugin)
> ```
> 
> This is because "LLVMPolly" is not a library, but a dummy "add_custom_target" 
> since loadable modules are not supported under Windows.
I'm no longer using `target_sources`, it should be fine now.



Comment at: llvm/docs/WritingAnLLVMPass.rst:1222
 
+Building out-of-tree passes
+===

Meinersbur wrote:
> "out-of-tree" is the wrong term. This registration only works if the plugin 
> if configured in the same cmake-run. "out-of-tree" would describe a processes 
> with a separate cmake source- and build-tree using `LLVM_MAIN_SRC_DIR` or 
> https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project
I'm not super happy with the new title, but I gave it a try.



Comment at: llvm/tools/CMakeLists.txt:45
 
+add_llvm_external_project(polly)
+

Meinersbur wrote:
> What is the reason to have this after `add_llvm_implicit_projects` in 
> contrast to the other LLVM subprojects?
polly used to be included before the other external projects, so that clang/opt 
could depend on it. We are now injecting dependencies a posteriori, so  polly 
needs to be included after opt/clang/bugpoint. I've put it right before the 
loop on `add_llvm_external_project` because they share the same function call 
name.



Comment at: polly/lib/Support/RegisterPasses.cpp:727
+extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
+llvmGetPassPluginInfo() {
+  return getPassPluginInfo();

Meinersbur wrote:
> serge-sans-paille wrote:
> > Meinersbur wrote:
> > > [serious] Unfortunately, the new pass manager's plugin system relies on 
> > > the function name to be `llvmGetPassPluginInfo` in each plugin. This 
> > > works with multiple dynamic libraries all declaring the same name using 
> > > the `PassPlugin::Load` mechanism, but linking them all statically will 
> > > violate the one-definition-rule.
> > > 
> > > IMHO, Polly.cpp would have been a better place for this function.
> > > but linking them all statically will violate the one-definition-rule.
> > 
> > They are unused when liked statically, and flagged as weak to avoid 
> > link-time conflict.
> > 
> > > IMHO, Polly.cpp would have been a better place for this function.
> > I still agree it's more explicit if linked conditionaly.
> You seem to have removed the weak attribute. Did you mean to put it into the 
> `polly` namespace to avoid name clashing with other plugins?
There are two entry points: `llvmGetPassPluginInfo`  for new PM (marked as 
weak) and `get##name##PluginInfo` for legacy PM (name is supposed to be unique, 
no need for weak linkage)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 206648.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

@Meinersbur your comment and my devs crossed, but this should be fine. This 
update enables new PM static plugin support for clang, something that was 
lacking to polly previously. I've tested it a bit and it builds fine with 
static setup, still need to test it with more config.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CMakeLists.txt
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/LLVMProcessSources.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp

[PATCH] D63613: [clang-tidy] Fail gracefully upon empty database fields

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Thanks @jkorous for the fast review o/


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63613



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

[PATCH] D63613: [clang-tidy] Fail gracefully upon empty database fields

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: alexfh.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.

fix https://bugs.llvm.org/show_bug.cgi?id=42281


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63613

Files:
  clang-tools-extra/test/clang-tidy/empty-database.cpp
  clang-tools-extra/test/clang-tidy/empty-database/compile_commands.json
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp


Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -481,7 +481,7 @@
   if (OverlayFileSystem->setCurrentWorkingDirectory(
   CompileCommand.Directory))
 llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(CompileCommand.Directory) + "\n!");
+ Twine(CompileCommand.Directory) + "\"!");
 
   // Now fill the in-memory VFS with the relative file mappings so it will
   // have the correct relative paths. We never remove mappings but that
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -150,7 +150,8 @@
 // spelling of each argument; re-rendering is lossy for aliased flags.
 // E.g. in CL mode, /W4 maps to -Wall.
 auto OptTable = clang::driver::createDriverOptTable();
-Cmd.CommandLine.emplace_back(OldArgs.front());
+if(!OldArgs.empty())
+  Cmd.CommandLine.emplace_back(OldArgs.front());
 for (unsigned Pos = 1; Pos < OldArgs.size();) {
   using namespace driver::options;
 
@@ -243,7 +244,7 @@
 }
 
 // Otherwise just check the clang executable file name.
-return llvm::sys::path::stem(CmdLine.front()).endswith_lower("cl");
+return !CmdLine.empty() && 
llvm::sys::path::stem(CmdLine.front()).endswith_lower("cl");
   }
 
   // Map the language from the --std flag to that of the -x flag.
Index: clang-tools-extra/test/clang-tidy/empty-database/compile_commands.json
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/empty-database/compile_commands.json
@@ -0,0 +1,4 @@
+[{
+"directory":"",
+"file":"/tmp/","arguments":[]
+}]
Index: clang-tools-extra/test/clang-tidy/empty-database.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/empty-database.cpp
@@ -0,0 +1,3 @@
+// RUN: not clang-tidy -p %S/empty-database %s 2>&1 | FileCheck %s
+
+// CHECK: LLVM ERROR: Cannot chdir into ""!


Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -481,7 +481,7 @@
   if (OverlayFileSystem->setCurrentWorkingDirectory(
   CompileCommand.Directory))
 llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(CompileCommand.Directory) + "\n!");
+ Twine(CompileCommand.Directory) + "\"!");
 
   // Now fill the in-memory VFS with the relative file mappings so it will
   // have the correct relative paths. We never remove mappings but that
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -150,7 +150,8 @@
 // spelling of each argument; re-rendering is lossy for aliased flags.
 // E.g. in CL mode, /W4 maps to -Wall.
 auto OptTable = clang::driver::createDriverOptTable();
-Cmd.CommandLine.emplace_back(OldArgs.front());
+if(!OldArgs.empty())
+  Cmd.CommandLine.emplace_back(OldArgs.front());
 for (unsigned Pos = 1; Pos < OldArgs.size();) {
   using namespace driver::options;
 
@@ -243,7 +244,7 @@
 }
 
 // Otherwise just check the clang executable file name.
-return llvm::sys::path::stem(CmdLine.front()).endswith_lower("cl");
+return !CmdLine.empty() && llvm::sys::path::stem(CmdLine.front()).endswith_lower("cl");
   }
 
   // Map the language from the --std flag to that of the -x flag.
Index: clang-tools-extra/test/clang-tidy/empty-database/compile_commands.json
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/empty-database/compile_commands.json
@@ -0,0 +1,4 @@
+[{
+"directory":"",
+"file":"/tmp/","arguments":[]
+}]
Index: clang-tools-extra/test/clang-tidy/empty-database.cpp
===
--- /dev/null
+++ 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 205298.
serge-sans-paille added a comment.

Mostly documentation update + helper function renaming.

@meinersbur, I've tested the setup with static and dynamic builds on Linux, 
please let me know your thoughts on this.


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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ polly/lib/Support/RegisterPasses.cpp
@@ -701,9 +701,24 @@
 }
 } // namespace polly
 
-// Plugin Entrypoint:
-extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
-llvmGetPassPluginInfo() {
+/// Initialize Polly passes when 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: polly/lib/Support/RegisterPasses.cpp:727
+extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
+llvmGetPassPluginInfo() {
+  return getPassPluginInfo();

Meinersbur wrote:
> [serious] Unfortunately, the new pass manager's plugin system relies on the 
> function name to be `llvmGetPassPluginInfo` in each plugin. This works with 
> multiple dynamic libraries all declaring the same name using the 
> `PassPlugin::Load` mechanism, but linking them all statically will violate 
> the one-definition-rule.
> 
> IMHO, Polly.cpp would have been a better place for this function.
> but linking them all statically will violate the one-definition-rule.

They are unused when liked statically, and flagged as weak to avoid link-time 
conflict.

> IMHO, Polly.cpp would have been a better place for this function.
I still agree it's more explicit if linked conditionaly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 204073.
serge-sans-paille marked 3 inline comments as done.
serge-sans-paille added a comment.

@beanz : In addition to your suggested changes, I've renamed the cmake utility 
into `add_llvm_pass_plugin` to match other functions from `AddLLVM.cmake`

@Meinersbur : thanks a lot for the polly layout explanation. I've slighlty 
updated your suggection like this:

  PollyCore.a:
RegisterPasses.o
polly_static_entry_point.cpp
  
  LLVMPolly.so (for use with -load mechanism)
RegisterPasses.o
polly_plugin_entry_point.cpp

initialization is done in both cases through a static initializer in 
`RegisterPasses.cpp`. `llvmGetPassPluginInfo` is only available in 
`polly_plugin_entry_point.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 203697.
serge-sans-paille added a comment.

Using @beanz idea, provide an option to `add_llvm_executable` to declare it as 
receiving plugins, which provides lower coupling with actual plugins

Tested with four configurations: `BUILD_SHARED_LIBS` ON/OFF and with 
llvm-project / legacy configuration, but *not* on Windows

As suggested by @Meinersbur, use static constructor for initialization step 
with legacy PM, for both shared and static libraries.

The patch looks much less intrusive now, thanks a lot to all reviewers for the 
provided guidance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Polly.cpp
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D61446#1532848 , @Meinersbur wrote:

> Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in 
> opt/bugpoint/clang by adding Polly.cpp as a source file or object library to 
> the executables. This would guarantee that its static initializer is called 
> without dynamic library.


That's what I tried to do when always adding Polly.cpp to PollyCore, but it 
doesn't seem to be enough, I still need to invstigate why (it is actually 
enough when using BUILD_SHARED_LIBS=ON)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Meinersbur Itested with the following configuration

  cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON 
-DCMAKE_BUILD_TYPE=Release  -DLLVM_TARGETS_TO_BUILD=X86
  
  cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON 
-DCMAKE_BUILD_TYPE=Release  -DLLVM_TARGETS_TO_BUILD=X86 
-DLLVM_POLLY_LINK_INTO_TOOLS=OFF

with old and mono repo layout. I'll test with BUILD_SHARED_LIBS=OFF too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D62623: Reduce memory consumption of coverage dumps

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4cd07dbeec98: Reduce memory consumption of coverage dumps 
(authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62623

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,19 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  // Stream the content of CoverageMappings to OS while keeping
+  // memory consumption under control.
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << S;
+S.clear();
+S.shrink_to_fit();
+  }
+  CoverageMappings.clear();
+  CoverageMappings.shrink_to_fit();
+
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,19 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  // Stream the content of CoverageMappings to OS while keeping
+  // memory consumption under control.
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << S;
+S.clear();
+S.shrink_to_fit();
+  }
+  CoverageMappings.clear();
+  CoverageMappings.shrink_to_fit();
+
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 203086.
serge-sans-paille added a comment.

- get Rid of the constraint on initializePasses : it can be registered suing a 
static constructor, no need to clutter the API with it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/CMakeLists.txt
  polly/include/polly/RegisterPasses.h
  polly/lib/CMakeLists.txt
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ polly/lib/Support/RegisterPasses.cpp
@@ -701,9 +701,13 @@
 }
 } // namespace polly
 
-// Plugin Entrypoint:
-extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK
-llvmGetPassPluginInfo() {
+// Pass Plugin Entrypoints

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 203029.
serge-sans-paille added a comment.

- make it possible for client code to speicify the registration function for 
new PM, thanks @philip.pfaffe for the hint. @Meinersbur this should be ok now.


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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/CMakeLists.txt
  polly/lib/CMakeLists.txt
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ polly/lib/Support/RegisterPasses.cpp
@@ -234,7 +234,6 @@
 cl::desc("Bail out on unprofitable SCoPs before rescheduling"), cl::Hidden,
 cl::init(true), cl::cat(PollyCategory));
 
-namespace polly {
 void initializePollyPasses(PassRegistry ) {
   

[PATCH] D62623: Reduce memory consumption of coverage dumps

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1393
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();

vsk wrote:
> It doesn't look like the CoverageMappings std::vector is needed at all. 
> Consider moving `FilenamesAndCoverageMappings` into CoverageMappingModuleGen?
> 
> That should reduce the memory requirements a bit more.
It's not possible to directly store into `FilenamesAndCoverageMappings` because 
filenames need to be stored before all mappings. It's possible to use a 
`std::string` instead of a `std::vector` but it means that at some point, 
memory usage will be `| CoverageMappings| x 2` while if we empty the vector 
elements as we push them to the stream, we should keep `|CoverageMappings|`. 
I've updated the diff to enforce this stream behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62623



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 202950.
serge-sans-paille added a comment.

Take into account @philip.pfaffe remarks and improve support for new PM.

- harmonize static/dynamic loading, although two functions are still needed, 
one for dynamic loading and one for static loading (new PM)
- do not clutter the required API with extra namespaces

I've updated https://sergesanspaille.fedorapeople.org/bye.tgz to showcase a 
minimal example.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/CMakeLists.txt
  polly/lib/Support/RegisterPasses.cpp
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/lib/Support/RegisterPasses.cpp
===
--- polly/lib/Support/RegisterPasses.cpp
+++ polly/lib/Support/RegisterPasses.cpp
@@ -234,7 

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@beanz turns out the depdendency problem was just a matter of moving the 
`register_llvm_extension` call at the top of the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@philip.pfaffe : NewPM integeration actually works thanks to 
``Register${Name}Passes(llvm::PassBuilder )``. It's already used by Polly 
for NewPM integration and we mimic that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 202908.
serge-sans-paille added a comment.

- reorder registration to make sure options are correctly taken into account
- make LINK_POLLY_INTO_TOOLS obsolete in favor of just using 
LLVM_LINK_POLLY_INTO_TOOLS


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/CMakeLists.txt
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in
  polly/test/update_check.py

Index: polly/test/update_check.py
===
--- polly/test/update_check.py
+++ polly/test/update_check.py
@@ -15,7 +15,7 @@
 polly_lib_dir = '''@POLLY_LIB_DIR@'''
 shlibext = '''@LLVM_SHLIBEXT@'''
 llvm_tools_dir = '''@LLVM_TOOLS_DIR@'''
-link_polly_into_tools = not '''@LINK_POLLY_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','link_polly_into_tools-notfound'}
+llvm_polly_link_into_tools = not '''@LLVM_POLLY_LINK_INTO_TOOLS@'''.lower() in {'','0','n','no','off','false','notfound','llvm_polly_link_into_tools-notfound'}
 
 runre = re.compile(r'\s*\;\s*RUN\s*\:(?P.*)')
 filecheckre = re.compile(r'\s*(?P.*)\|\s*(?PFileCheck\s[^|]*)')
@@ -298,7 +298,7 @@
 toolarg = toolarg.replace('%s', filename)
 toolarg = toolarg.replace('%S', os.path.dirname(filename))
 if toolarg == '%loadPolly':
-if not link_polly_into_tools:
+if not llvm_polly_link_into_tools:
 newtool += ['-load',os.path.join(polly_lib_dir,'LLVMPolly' + shlibext)]
 newtool.append('-polly-process-unprofitable')
 newtool.append('-polly-remarks-minimal')
Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -8,7 +8,7 @@
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.targets_to_build = "@TARGETS_TO_BUILD@"
 config.extra_paths = "@POLLY_TEST_EXTRA_PATHS@".split(";")
 
@@ -36,14 +36,14 @@
 # directories.
 config.excludes = ['Inputs']
 
-if config.link_polly_into_tools == '' or \
-   config.link_polly_into_tools.lower() == '0' or \
-   config.link_polly_into_tools.lower() == 'n' or \
-   config.link_polly_into_tools.lower() == 'no' or \
-   config.link_polly_into_tools.lower() == 'off' or \
-   config.link_polly_into_tools.lower() == 'false' or \
-   config.link_polly_into_tools.lower() == 'notfound' or \
-   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':
+if config.llvm_polly_link_into_tools == '' or \
+   config.llvm_polly_link_into_tools.lower() == '0' or \
+   config.llvm_polly_link_into_tools.lower() == 'n' or \
+   config.llvm_polly_link_into_tools.lower() == 'no' or \
+   config.llvm_polly_link_into_tools.lower() == 'off' or \
+   config.llvm_polly_link_into_tools.lower() == 'false' or \
+   config.llvm_polly_link_into_tools.lower() == 'notfound' or \
+   config.llvm_polly_link_into_tools.lower() == 'llvm_polly_link_into_tools-notfound':
 config.substitutions.append(('%loadPolly', '-load '
  + config.polly_lib_dir + '/LLVMPolly@LLVM_SHLIBEXT@'
  + ' -load-pass-plugin '
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -13,7 +13,7 @@
 config.shlibdir = "@SHLIBDIR@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.enable_gpgpu_codegen = "@GPU_CODEGEN@"
-config.link_polly_into_tools = "@LINK_POLLY_INTO_TOOLS@"
+config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
 # Support substitution of the tools_dir, libs_dirs, and build_mode with user
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -162,6 +162,11 @@
 
 add_definitions( -D_GNU_SOURCE )
 
+
+register_llvm_extension(Polly)
+# Polly-ACC requires the NVPTX target to be present in the executable it is linked to
+set_property(TARGET bugpoint APPEND PROPERTY LLVM_COMPILER_EXTENSIONS LLVMTarget)
+
 add_subdirectory(docs)
 add_subdirectory(lib)
 

[PATCH] D62623: Reduce memory consumption of coverage dumps

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 202896.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

Update comment + force reduced memory consumption.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62623

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,19 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  // Stream the content of CoverageMappings to OS while keeping
+  // memory consumption under control.
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << S;
+S.clear();
+S.shrink_to_fit();
+  }
+  CoverageMappings.clear();
+  CoverageMappings.shrink_to_fit();
+
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,19 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  // Stream the content of CoverageMappings to OS while keeping
+  // memory consumption under control.
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << S;
+S.clear();
+S.shrink_to_fit();
+  }
+  CoverageMappings.clear();
+  CoverageMappings.shrink_to_fit();
+
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 202795.
serge-sans-paille added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- provide minimal documentation
- fix testing configuration
- validate locally with both shared and static libraries, using monorepo or 
tool layout
- update sample https://sergesanspaille.fedorapeople.org/bye.tgz

@Meinersbur thanks a lot for the previous review! lld integration, as well as 
new PM integration, will be for another commit, in order to prevent this review 
from growing too large


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446

Files:
  clang/tools/driver/CMakeLists.txt
  clang/tools/driver/cc1_main.cpp
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/docs/WritingAnLLVMPass.rst
  llvm/include/llvm/Config/llvm-config.h.cmake
  llvm/tools/CMakeLists.txt
  llvm/tools/bugpoint/CMakeLists.txt
  llvm/tools/bugpoint/bugpoint.cpp
  llvm/tools/opt/CMakeLists.txt
  llvm/tools/opt/NewPMDriver.cpp
  llvm/tools/opt/opt.cpp
  llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -208,3 +208,7 @@
 add_custom_target(polly-update-format DEPENDS ${update_format_depends})
 set_target_properties(polly-update-format PROPERTIES FOLDER "Polly")
 
+register_llvm_extension(polly)
+
+# Polly-ACC requires the NVPTX target to be present in the executable it is linked to
+set_property(TARGET bugpoint APPEND PROPERTY LLVM_COMPILER_EXTENSIONS LLVMTarget)
Index: llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -321,7 +321,6 @@
   output = "$target_gen_dir/llvm-config.h"
   values = [
 "LLVM_ENABLE_DUMP=",
-"LINK_POLLY_INTO_TOOLS=",
 "LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
 "LLVM_HAS_ATOMICS=1",
 "LLVM_HOST_TRIPLE=$llvm_current_triple",
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -470,11 +470,11 @@
 getCodeModel(), GetCodeGenOptLevel());
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void initializePollyPasses(llvm::PassRegistry );
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext)  \
+  namespace Ns {   \
+  void initialize##Ext##Passes(llvm::PassRegistry );  \
+  }
+#include "llvm/Support/Extension.def"
 
 //===--===//
 // main for opt
@@ -529,9 +529,8 @@
   initializeWasmEHPreparePass(Registry);
   initializeWriteBitcodePassPass(Registry);
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  polly::initializePollyPasses(Registry);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) Ns::initialize##Ext##Passes(Registry);
+#include "llvm/Support/Extension.def"
 
   cl::ParseCommandLineOptions(argc, argv,
 "llvm .bc -> .bc modular optimizer and analysis printer\n");
Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -202,11 +202,11 @@
 });
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void RegisterPollyPasses(PassBuilder &);
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext)  \
+  namespace Ns {   \
+  void Register##Ext##Passes(PassBuilder &);   \
+  }
+#include "llvm/Support/Extension.def"
 
 bool llvm::runPassPipeline(StringRef Arg0, Module , TargetMachine *TM,
ToolOutputFile *Out, ToolOutputFile *ThinLTOLinkOut,
@@ -290,9 +290,8 @@
 return false;
   });
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  polly::RegisterPollyPasses(PB);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) Ns::Register##Ext##Passes(PB);
+#include "llvm/Support/Extension.def"
 
   // Specially handle the alias analysis manager so that we can register
   // a custom pipeline of AA passes with it.
Index: llvm/tools/opt/CMakeLists.txt
===
--- llvm/tools/opt/CMakeLists.txt
+++ llvm/tools/opt/CMakeLists.txt
@@ -37,7 +37,4 @@
   intrinsics_gen
   )
 export_executable_symbols(opt)
-
-if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
-  target_link_libraries(opt PRIVATE Polly)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
+target_link_libraries(opt PRIVATE $)
Index: llvm/tools/bugpoint/bugpoint.cpp

[PATCH] D62623: Reduce memory consumption of coverage dumps

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@kwk: looks like you're still compiling with clang-7, this patch is to be 
applied on the master version of LLVM/clang, then you can install it and use it 
to recompile llvm/lld with coverage info.

Does it make sense to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62623



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

[PATCH] D62623: Reduce memory consumption of coverage dumps

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: vsk, arphaman.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
serge-sans-paille edited the summary of this revision.

Avoiding an intermediate join operation, which in turns removes the need for an
intermediate buffer that may be quite large, as showcased by

  https://bugs.llvm.org/show_bug.cgi?id=41965


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62623

Files:
  clang/lib/CodeGen/CoverageMappingGen.cpp


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,12 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << std::move(S);
+  }
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.


Index: clang/lib/CodeGen/CoverageMappingGen.cpp
===
--- clang/lib/CodeGen/CoverageMappingGen.cpp
+++ clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -1388,10 +1388,12 @@
   std::string FilenamesAndCoverageMappings;
   llvm::raw_string_ostream OS(FilenamesAndCoverageMappings);
   CoverageFilenamesSectionWriter(FilenameRefs).write(OS);
-  std::string RawCoverageMappings =
-  llvm::join(CoverageMappings.begin(), CoverageMappings.end(), "");
-  OS << RawCoverageMappings;
-  size_t CoverageMappingSize = RawCoverageMappings.size();
+
+  size_t CoverageMappingSize = 0;
+  for (auto  : CoverageMappings) {
+CoverageMappingSize += S.size();
+OS << std::move(S);
+  }
   size_t FilenamesSize = OS.str().size() - CoverageMappingSize;
   // Append extra zeroes if necessary to ensure that the size of the filenames
   // and coverage mappings is a multiple of 8.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105
   // which is just that always inlining occurs.
-  MPM.addPass(AlwaysInlinerPass());
+  // We always pass false here since according to the legacy PM logic for
+  // enabling lifetime intrinsics, we should not be compiling with O0.
+  MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));

echristo wrote:
> Can you elaborate more here? We do turn on the always inliner at O0 which 
> makes this comment a bit confusing.
I guess he means 

We always pass false here since according to the legacy PM logic for 
enabling lifetime intrinsics, they are not required with O0



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62225



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

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 199585.
serge-sans-paille added a comment.

Updated with clang-formated diff


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

https://reviews.llvm.org/D59413

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/test/Misc/no-warn-in-system-macro.c


Index: clang/test/Misc/no-warn-in-system-macro.c
===
--- clang/test/Misc/no-warn-in-system-macro.c
+++ clang/test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
double foo = 1.0;
 
if (isnan(foo))
return 1;
-   return 0;
+
+MACRO(isnan(foo));
+
+return 0;
 }
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -1466,8 +1466,13 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do {
+loc = getImmediateMacroCallerLoc(loc);
+  } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }


Index: clang/test/Misc/no-warn-in-system-macro.c
===
--- clang/test/Misc/no-warn-in-system-macro.c
+++ clang/test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
 	double foo = 1.0;
 
 	if (isnan(foo))
 		return 1;
-	return 0;
+
+MACRO(isnan(foo));
+
+return 0;
 }
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -1466,8 +1466,13 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do {
+loc = getImmediateMacroCallerLoc(loc);
+  } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith : up :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59413



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

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith : up :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59413



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

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith up!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59413



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

[PATCH] D59734: [clang-tidy] Handle missing yaml module in run-clang-tidy.py

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang-tidy/tool/run-clang-tidy.py:210
+help='Create a yaml file to store suggested fixes in, '
+'which can be applied with clang-apply-replacements.')
   parser.add_argument('-j', type=int, default=0,

could be

```
if yaml:
parser.add_argument('-export-fixes', metavar='filename', 
dest='export_fixes',
​help='Create a yaml file to store suggested 
fixes in, '
​'which can be applied with 
clang-apply-replacements.')
else:
parser.export_fixes = False
```

This would make later condition easier to read.


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

https://reviews.llvm.org/D59734



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

[PATCH] D59734: [clang-tidy] Handle missing yaml module in run-clang-tidy.py

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang-tidy/tool/run-clang-tidy.py:51
+
+yaml_imported = True
+try:

Could be

```
try:
import yaml
except ImportError:
yaml = None
```

and later

```
if yaml:
...
```


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

https://reviews.llvm.org/D59734



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

[PATCH] D59440: add steps to preprocess file and reduce command line args

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:45
+  result = []
+  for i, arg in enumerate(cmd):
+if arg.startswith('$'):

Useless enumerate here. You could even use a list comprehension here

```
return ' '.join(arg if arg.startswith('$') else pipes.quote(arg) for arg in cmd)
```



Comment at: clang/utils/creduce-clang-crash.py:161
+  for arg in args:
+if (any(arg.startswith(a) for a in opts_startswith)):
+  continue

useless parenthesis around test

could be a list comprehension

```
result = [arg for arg in args if all(not arg.startswith(a) for a in 
opts_startswith)]
```


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

https://reviews.llvm.org/D59440



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

[PATCH] D59413: Fix isInSystemMacro in presence of macro and pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When a warning is raised from the expansion of a  system macro that involves 
pasted token, there was still situations were they were not skipped, as 
showcased by this issue: https://bugzilla.redhat.com/show_bug.cgi?id=1472437


Repository:
  rC Clang

https://reviews.llvm.org/D59413

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/test/Misc/no-warn-in-system-macro.c


Index: clang/test/Misc/no-warn-in-system-macro.c
===
--- clang/test/Misc/no-warn-in-system-macro.c
+++ clang/test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
double foo = 1.0;
 
if (isnan(foo))
return 1;
+
+   MACRO(isnan(foo));
+
return 0;
 }
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -1463,8 +1463,11 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do { loc = getImmediateMacroCallerLoc(loc); } while 
(isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }


Index: clang/test/Misc/no-warn-in-system-macro.c
===
--- clang/test/Misc/no-warn-in-system-macro.c
+++ clang/test/Misc/no-warn-in-system-macro.c
@@ -3,11 +3,16 @@
 
 #include 
 
+#define MACRO(x) x
+
 int main(void)
 {
 	double foo = 1.0;
 
 	if (isnan(foo))
 		return 1;
+
+	MACRO(isnan(foo));
+
 	return 0;
 }
Index: clang/include/clang/Basic/SourceManager.h
===
--- clang/include/clang/Basic/SourceManager.h
+++ clang/include/clang/Basic/SourceManager.h
@@ -1463,8 +1463,11 @@
 
 // This happens when the macro is the result of a paste, in that case
 // its spelling is the scratch memory, so we take the parent context.
-if (isWrittenInScratchSpace(getSpellingLoc(loc)))
-  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+// There can be several level of token pasting.
+if (isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  do { loc = getImmediateMacroCallerLoc(loc); } while (isWrittenInScratchSpace(getSpellingLoc(loc)));
+  return isInSystemMacro(loc);
+}
 
 return isInSystemHeader(getSpellingLoc(loc));
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57523: Fix uninitialized value in ABIArgInfo

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@hans agreed; Thanks for taking the time to try to reproduce the original issue 
o/


Repository:
  rC Clang

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

https://reviews.llvm.org/D57523



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

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@MarcoFalke it's in, thanks for the patch o/


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57528



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

[PATCH] D57528: [tools] Fix python DeprecationWarning: invalid escape sequence

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.
Herald added a project: clang.

LGTM, do you have commit right?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57528



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

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.
Herald added a subscriber: llvm-commits.

It's in, thanks @rsmith for the review.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55782



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

[PATCH] D57523: Fix uninitialized value in ABIArgInfo

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: hans.
Herald added a subscriber: cfe-commits.

GCC-9 takes advantage of this uninitialized values to optimize stuff, which 
ends up in failing validation when compiling clang.

This fixes https://bugs.llvm.org/show_bug.cgi?id=40547


Repository:
  rC Clang

https://reviews.llvm.org/D57523

Files:
  include/clang/CodeGen/CGFunctionInfo.h


Index: include/clang/CodeGen/CGFunctionInfo.h
===
--- include/clang/CodeGen/CGFunctionInfo.h
+++ include/clang/CodeGen/CGFunctionInfo.h
@@ -111,14 +111,14 @@
   }
 
   ABIArgInfo(Kind K)
-  : TheKind(K), PaddingInReg(false), InReg(false), SuppressSRet(false) {
+  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
+TheKind(K), PaddingInReg(false), InAllocaSRet(false), 
IndirectByVal(false),
+IndirectRealign(false), SRetAfterThis(false), InReg(false),
+CanBeFlattened(false), SignExt(false), SuppressSRet(false) {}
   }
 
 public:
-  ABIArgInfo()
-  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-TheKind(Direct), PaddingInReg(false), InReg(false),
-SuppressSRet(false) {}
+  ABIArgInfo() : ABIArgInfo(Direct) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
   llvm::Type *Padding = nullptr,


Index: include/clang/CodeGen/CGFunctionInfo.h
===
--- include/clang/CodeGen/CGFunctionInfo.h
+++ include/clang/CodeGen/CGFunctionInfo.h
@@ -111,14 +111,14 @@
   }
 
   ABIArgInfo(Kind K)
-  : TheKind(K), PaddingInReg(false), InReg(false), SuppressSRet(false) {
+  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
+TheKind(K), PaddingInReg(false), InAllocaSRet(false), IndirectByVal(false),
+IndirectRealign(false), SRetAfterThis(false), InReg(false),
+CanBeFlattened(false), SignExt(false), SuppressSRet(false) {}
   }
 
 public:
-  ABIArgInfo()
-  : TypeData(nullptr), PaddingType(nullptr), DirectOffset(0),
-TheKind(Direct), PaddingInReg(false), InReg(false),
-SuppressSRet(false) {}
+  ABIArgInfo() : ABIArgInfo(Direct) {}
 
   static ABIArgInfo getDirect(llvm::Type *T = nullptr, unsigned Offset = 0,
   llvm::Type *Padding = nullptr,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.



Comment at: include/clang/Basic/SourceManager.h:1458-1460
+   // This happens when the macro is the result of a paste, in that 
case
+   // its spelling is the scratch memory, so we take the parent 
context.
+isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc;

rsmith wrote:
> I think this also matches things like:
> 
> ```
> // system header
> #define BAR FOO
> 
> // user code
> #define FOO x
> BAR
> ```
> 
> ... where the location of `x` token produced by the `BAR` macro will now be 
> considered to be in a system macro. Can you make this more targeted? (Maybe 
> we could change the `SLocEntry` for the token-paste scratch buffer to be 
> considered a system header if the `##` token is in a system macro -- that'll 
> need the preprocessor to store two `ScratchBuffer`s, one for system headers 
> and one for user code, but that seems acceptable.)
Thanks for the advice! This approach should be more efficient and avoid the 
situation you point out - I've added a test case to illustrate that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55782



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

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 184275.
serge-sans-paille added a comment.

Patch updated to take into account @rsmith remarks. Test case added as an 
illustration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55782

Files:
  include/clang/Basic/SourceManager.h
  test/Misc/no-warn-in-system-macro.c
  test/Misc/no-warn-in-system-macro.c.inc
  test/Misc/warn-in-system-macro-def.c
  test/Misc/warn-in-system-macro-def.c.inc

Index: test/Misc/warn-in-system-macro-def.c.inc
===
--- /dev/null
+++ test/Misc/warn-in-system-macro-def.c.inc
@@ -0,0 +1,4 @@
+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define ISNAN isnan
Index: test/Misc/warn-in-system-macro-def.c
===
--- /dev/null
+++ test/Misc/warn-in-system-macro-def.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | FileCheck -allow-empty %s
+// CHECK: warning:
+// CHECK: expanded from macro 'ISNAN'
+// CHECK: expanded from macro 'isnan'
+
+#include 
+
+#define isnan(x) \
+	(sizeof (x) == sizeof (float)\
+	? __isnanf (x)\
+	: sizeof (x) == sizeof (double)   \
+	? __isnan (x) : __isnanl (x))
+
+int main(void)
+{
+	double foo = 1.0;
+
+	if (ISNAN(foo))
+		return 1;
+	return 0;
+}
Index: test/Misc/no-warn-in-system-macro.c.inc
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c.inc
@@ -0,0 +1,9 @@
+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define isnan(x) \
+	(sizeof (x) == sizeof (float)\
+	? __isnanf (x)\
+	: sizeof (x) == sizeof (double)   \
+	? __isnan (x) : __isnanl (x))
+
Index: test/Misc/no-warn-in-system-macro.c
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | FileCheck -allow-empty %s
+// CHECK-NOT: warning:
+
+#include 
+
+int main(void)
+{
+	double foo = 1.0;
+
+	if (isnan(foo))
+		return 1;
+	return 0;
+}
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1440,6 +1440,12 @@
 return Filename.equals("");
   }
 
+  /// Returns whether \p Loc is located in a  file.
+  bool isWrittenInScratchSpace(SourceLocation Loc) const {
+StringRef Filename(getPresumedLoc(Loc).getFilename());
+return Filename.equals("");
+  }
+
   /// Returns if a SourceLocation is in a system header.
   bool isInSystemHeader(SourceLocation Loc) const {
 return isSystem(getFileCharacteristic(Loc));
@@ -1452,7 +1458,17 @@
 
   /// Returns whether \p Loc is expanded from a macro in a system header.
   bool isInSystemMacro(SourceLocation loc) const {
-return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
+if(!loc.isMacroID())
+  return false;
+
+// This happens when the macro is the result of a paste, in that case
+// its spelling is the scratch memory, so we take the parent context.
+if(isWrittenInScratchSpace(getSpellingLoc(loc))) {
+  return isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc)));
+}
+else {
+  return isInSystemHeader(getSpellingLoc(loc));
+}
   }
 
   /// The size of the SLocEntry that \p FID represents.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56429: fix python3 compability issue

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Thanks @roxma for the patch!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56429



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

[PATCH] D56429: fix python3 compability issue

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@roxma: do you want me to commit this on your behalf?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56429



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

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith up!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55782



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

[PATCH] D56429: fix python3 compability issue

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added inline comments.
This revision is now accepted and ready to land.



Comment at: bindings/python/clang/cindex.py:2998
+for i,(name,contents) in enumerate(unsaved_files):
+if hasattr(contents, "read"):
+contents = contents.read()

roxma wrote:
> serge-sans-paille wrote:
> > serge-sans-paille wrote:
> > > Why did you remove the FIXME comment?
> > @roxma LGTM except this FIXME removal.
> @serge-sans-paille
> 
> the `contents.read()` looks almost the same as line 2817.
> 
> It is better to keep the code consistent. The FIXME doesn't seem to be 
> helpful.
ok, I'll trust you on this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56429



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

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 181498.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

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

https://reviews.llvm.org/D55782

Files:
  include/clang/Basic/SourceManager.h
  test/Misc/no-warn-in-system-macro.c
  test/Misc/no-warn-in-system-macro.c.inc


Index: test/Misc/no-warn-in-system-macro.c.inc
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c.inc
@@ -0,0 +1,9 @@
+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define isnan(x) \
+   (sizeof (x) == sizeof (float)\
+   ? __isnanf (x)\
+   : sizeof (x) == sizeof (double)   \
+   ? __isnan (x) : __isnanl (x))
+
Index: test/Misc/no-warn-in-system-macro.c
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | 
FileCheck -allow-empty %s
+// CHECK-NOT: warning:
+
+#include 
+
+int main(void)
+{
+   double foo = 1.0;
+
+   if (isnan(foo))
+   return 1;
+   return 0;
+}
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1453,7 +1453,11 @@
 
   /// Returns whether \p Loc is expanded from a macro in a system header.
   bool isInSystemMacro(SourceLocation loc) const {
-return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
+return loc.isMacroID() &&
+   (isInSystemHeader(getSpellingLoc(loc)) ||
+   // This happens when the macro is the result of a paste, in that 
case
+   // its spelling is the scratch memory, so we take the parent 
context.
+isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc;
   }
 
   /// The size of the SLocEntry that \p FID represents.


Index: test/Misc/no-warn-in-system-macro.c.inc
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c.inc
@@ -0,0 +1,9 @@
+extern int __isnanf(float f);
+extern int __isnan(double f);
+extern int __isnanl(long double f);
+#define isnan(x) \
+	(sizeof (x) == sizeof (float)\
+	? __isnanf (x)\
+	: sizeof (x) == sizeof (double)   \
+	? __isnan (x) : __isnanl (x))
+
Index: test/Misc/no-warn-in-system-macro.c
===
--- /dev/null
+++ test/Misc/no-warn-in-system-macro.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -isystem %S -Wdouble-promotion -fsyntax-only %s  2>&1 | FileCheck -allow-empty %s
+// CHECK-NOT: warning:
+
+#include 
+
+int main(void)
+{
+	double foo = 1.0;
+
+	if (isnan(foo))
+		return 1;
+	return 0;
+}
Index: include/clang/Basic/SourceManager.h
===
--- include/clang/Basic/SourceManager.h
+++ include/clang/Basic/SourceManager.h
@@ -1453,7 +1453,11 @@
 
   /// Returns whether \p Loc is expanded from a macro in a system header.
   bool isInSystemMacro(SourceLocation loc) const {
-return loc.isMacroID() && isInSystemHeader(getSpellingLoc(loc));
+return loc.isMacroID() &&
+   (isInSystemHeader(getSpellingLoc(loc)) ||
+	// This happens when the macro is the result of a paste, in that case
+	// its spelling is the scratch memory, so we take the parent context.
+isInSystemHeader(getSpellingLoc(getImmediateMacroCallerLoc(loc;
   }
 
   /// The size of the SLocEntry that \p FID represents.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56429: fix python3 compability issue

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: bindings/python/clang/cindex.py:2998
+for i,(name,contents) in enumerate(unsaved_files):
+if hasattr(contents, "read"):
+contents = contents.read()

serge-sans-paille wrote:
> Why did you remove the FIXME comment?
@roxma LGTM except this FIXME removal.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56429



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

[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54071



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

[PATCH] D54071: [Bug 39548][Clang] PGO bootstrap fails with python3: errors in perf-helper.py

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@Romain-Geissler-1A sure thing. Can you rebase your patch on master first?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54071



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

[PATCH] D56429: fix python3 compability issue

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: bindings/python/clang/cindex.py:2998
+for i,(name,contents) in enumerate(unsaved_files):
+if hasattr(contents, "read"):
+contents = contents.read()

Why did you remove the FIXME comment?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56429



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

[PATCH] D56343: [clang-tidy] Refactor: Compose Method on check_clang_tidy.py

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: test/clang-tidy/check_clang_tidy.py:103
 
+  return (has_check_fixes, has_check_messages, has_check_notes, \
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes)

Such a long list of returned values make me wonder if it would be worth 
creating a NamedTuple / class to hold them.
This may make the call site more readable too.



Comment at: test/clang-tidy/check_clang_tidy.py:141
+   '-check-prefixes=' + ','.join(check_notes_prefixes),
+   '-implicit-check-not={{note|warning|error}}:'])
+  return

These three `check_*` function all use the same `'FileCheck', '-']` 
pattern. Maybe it's worth syndicating that to a try_run_filecheck(input_file0, 
input_file1, *extra_args)` function.



Comment at: test/clang-tidy/check_clang_tidy.py:178
+check_fixes_prefixes, check_messages_prefixes, check_notes_prefixes = \
+get_prefixes(args.check_suffix, input_text)
+

This is the verbose call site I was pointing to above.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56343



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

[PATCH] D56341: [python] Make the collections import future-proof

[serge via Phabricator via cfe-commits]

serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D56341



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

[PATCH] D55382: Make -Wstring-plus-int warns even if when the result is not out of bounds

[serge via Phabricator via cfe-commits]

serge-sans-paille resigned from this revision.
serge-sans-paille added a comment.

Just wanted to review the change to the Python script to ensure it remains 
compatible with Python2 and Python3, which is indeed the case; so LGTM on that 
aspect.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55382



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 176330.
serge-sans-paille edited the summary of this revision.

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

https://reviews.llvm.org/D55121

Files:
  bindings/python/clang/cindex.py
  docs/tools/dump_format_style.py
  tools/scan-view/share/Reporter.py
  tools/scan-view/share/ScanView.py
  utils/ABITest/ABITestGen.py
  utils/ABITest/TypeGen.py
  utils/analyzer/CmpRuns.py
  utils/modfuzz.py
  utils/token-delta.py

Index: utils/token-delta.py
===
--- utils/token-delta.py
+++ utils/token-delta.py
@@ -94,7 +94,7 @@
 
 ###
 
-class Token:
+class Token(object):
 def __init__(self, type, data, flags, file, line, column):
 self.type   = type
 self.data   = data
Index: utils/modfuzz.py
===
--- utils/modfuzz.py
+++ utils/modfuzz.py
@@ -12,7 +12,7 @@
 clang = sys.argv[1]
 none_opts = 0.3
 
-class Decl:
+class Decl(object):
   def __init__(self, text, depends=[], provides=[], conflicts=[]):
 self.text = text
 self.depends = depends
@@ -39,7 +39,7 @@
   Decl('X %(name)s;\n', depends=['X']),
 ]
 
-class FS:
+class FS(object):
   def __init__(self):
 self.fs = {}
 self.prevfs = {}
@@ -62,7 +62,7 @@
 
 fs = FS()
 
-class CodeModel:
+class CodeModel(object):
   def __init__(self):
 self.source = ''
 self.modules = {}
Index: utils/analyzer/CmpRuns.py
===
--- utils/analyzer/CmpRuns.py
+++ utils/analyzer/CmpRuns.py
@@ -38,7 +38,7 @@
 
 STATS_REGEXP = re.compile(r"Statistics: (\{.+\})", re.MULTILINE | re.DOTALL)
 
-class Colors:
+class Colors(object):
 """
 Color for terminal highlight.
 """
@@ -50,14 +50,14 @@
 # path - the analysis output directory
 # root - the name of the root directory, which will be disregarded when
 # determining the source file name
-class SingleRunInfo:
+class SingleRunInfo(object):
 def __init__(self, path, root="", verboseLog=None):
 self.path = path
 self.root = root.rstrip("/\\")
 self.verboseLog = verboseLog
 
 
-class AnalysisDiagnostic:
+class AnalysisDiagnostic(object):
 def __init__(self, data, report, htmlReport):
 self._data = data
 self._loc = self._data['location']
@@ -117,14 +117,14 @@
 return self._data
 
 
-class AnalysisReport:
+class AnalysisReport(object):
 def __init__(self, run, files):
 self.run = run
 self.files = files
 self.diagnostics = []
 
 
-class AnalysisRun:
+class AnalysisRun(object):
 def __init__(self, info):
 self.path = info.path
 self.root = info.root
Index: utils/ABITest/TypeGen.py
===
--- utils/ABITest/TypeGen.py
+++ utils/ABITest/TypeGen.py
@@ -17,7 +17,7 @@
 ###
 # Actual type types
 
-class Type:
+class Type(object):
 def isBitField(self):
 return False
 
Index: utils/ABITest/ABITestGen.py
===
--- utils/ABITest/ABITestGen.py
+++ utils/ABITest/ABITestGen.py
@@ -10,7 +10,7 @@
 
 
 
-class TypePrinter:
+class TypePrinter(object):
 def __init__(self, output, outputHeader=None, 
  outputTests=None, outputDriver=None,
  headerName=None, info=None):
Index: tools/scan-view/share/ScanView.py
===
--- tools/scan-view/share/ScanView.py
+++ tools/scan-view/share/ScanView.py
@@ -422,7 +422,7 @@
 return self.send_string(res, 'text/plain')
 
 def get_report_context(self, report):
-class Context:
+class Context(object):
 pass
 if report is None or report == 'None':
 data = self.load_crashes()
Index: tools/scan-view/share/Reporter.py
===
--- tools/scan-view/share/Reporter.py
+++ tools/scan-view/share/Reporter.py
@@ -16,7 +16,7 @@
 
 # Collect information about a bug.
 
-class BugReport:
+class BugReport(object):
 def __init__(self, title, description, files):
 self.title = title
 self.description = description
@@ -37,7 +37,7 @@
 # ReporterParameter
 #======#
 
-class ReporterParameter:
+class ReporterParameter(object):
   def __init__(self, n):
 self.name = n
   def getName(self):
@@ -75,7 +75,7 @@
 # Reporters
 #======#
 
-class EmailReporter:
+class EmailReporter(object):
 def getName(self):
 return 'Email'
 
@@ -143,7 +143,7 @@
 
 return "Message sent!"
 
-class BugzillaReporter:
+class BugzillaReporter(object):
 def getName(self):
 return 'Bugzilla'
 
@@ -174,7 +174,7 @@
 else:
   return '7'

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 176141.
serge-sans-paille added a comment.

Use generic ``from __future__ import `` line whenever it makes sense


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

https://reviews.llvm.org/D55121

Files:
  bindings/python/clang/cindex.py
  bindings/python/examples/cindex/cindex-dump.py
  bindings/python/tests/cindex/test_translation_unit.py
  docs/conf.py
  docs/tools/dump_format_style.py
  tools/clang-format/clang-format-diff.py
  tools/clang-format/clang-format-sublime.py
  tools/clang-format/clang-format.py
  tools/clang-format/git-clang-format
  tools/clang-rename/clang-rename.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-view/share/Reporter.py
  utils/ABITest/ABITestGen.py
  utils/ABITest/Enumeration.py
  utils/CIndex/completion_logger_server.py
  utils/TestUtils/deep-stack.py
  utils/analyzer/SATestAdd.py
  utils/analyzer/SATestUpdateDiffs.py
  utils/analyzer/SumTimerInfo.py
  utils/check_cfc/check_cfc.py
  utils/check_cfc/obj_diff.py
  utils/check_cfc/setup.py
  utils/check_cfc/test_check_cfc.py
  utils/clangdiag.py
  utils/hmaptool/hmaptool
  utils/modfuzz.py
  utils/perf-training/perf-helper.py
  utils/token-delta.py
  www/builtins.py

Index: www/builtins.py
===
--- www/builtins.py
+++ www/builtins.py
@@ -151,7 +151,7 @@
   sys.stderr.write("%s:%d: x86 builtin %s used, too many replacements\n" % (fileinput.filename(), fileinput.filelineno(), builtin))
 
 for line in fileinput.input(inplace=1):
-  for builtin, repl in repl_map.iteritems():
+  for builtin, repl in repl_map.items():
 if builtin in line:
   line = line.replace(builtin, repl)
   report_repl(builtin, repl)
Index: utils/token-delta.py
===
--- utils/token-delta.py
+++ utils/token-delta.py
@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 
+from __future__ import absolute_import, division, print_function
 import os
 import re
 import subprocess
@@ -37,7 +38,7 @@
 # O(N^2) case unless user requests it.
 if not force:
 if not self.getTestResult(changes):
-raise ValueError,'Initial test passed to delta fails.'
+raise ValueError('Initial test passed to delta fails.')
 
 # Check empty set first to quickly find poor test functions.
 if self.getTestResult(set()):
@@ -94,7 +95,7 @@
 
 ###
 
-class Token:
+class Token(object):
 def __init__(self, type, data, flags, file, line, column):
 self.type   = type
 self.data   = data
@@ -165,7 +166,7 @@
 byFile = self.writeFiles(changes, self.tempFiles)
 
 if self.log:
-print >>sys.stderr, 'TEST - ',
+print('TEST - ', end=' ', file=sys.stderr)
 if self.log > 1:
 for i,(file,_) in enumerate(self.tokenLists):
 indices = byFile[i]
@@ -184,8 +185,8 @@
 sys.stderr.write(str(byFile[i][-1]))
 sys.stderr.write('] ')
 else:
-print >>sys.stderr, ', '.join(['%s:%d tokens' % (file, len(byFile[i]))
-   for i,(file,_) in enumerate(self.tokenLists)]),
+print(', '.join(['%s:%d tokens' % (file, len(byFile[i]))
+   for i,(file,_) in enumerate(self.tokenLists)]), end=' ', file=sys.stderr)
 
 p = subprocess.Popen([self.testProgram] + self.tempFiles)
 res = p.wait() == 0
@@ -194,10 +195,10 @@
 self.writeFiles(changes, self.targetFiles)
 
 if self.log:
-print >>sys.stderr, '=> %s' % res
+print('=> %s' % res, file=sys.stderr)
 else:
 if res:
-print '\nSUCCESS (%d tokens)' % len(changes)
+print('\nSUCCESS (%d tokens)' % len(changes))
 else:
 sys.stderr.write('.')
 
@@ -209,7 +210,7 @@
   for j in range(len(tokens))])
 self.writeFiles(res, self.targetFiles)
 if not self.log:
-print >>sys.stderr
+print(file=sys.stderr)
 return res
 
 def tokenBasedMultiDelta(program, files, log):
@@ -218,15 +219,15 @@
   for file in files]
 
 numTokens = sum([len(tokens) for _,tokens in tokenLists])
-print "Delta on %s with %d tokens." % (', '.join(files), numTokens)
+print("Delta on %s with %d tokens." % (', '.join(files), numTokens))
 
 tbmd = TMBDDelta(program, tokenLists, log)
 
 res = tbmd.run()
 
-print "Finished %s with %d tokens (in %d tests)." % (', '.join(tbmd.targetFiles),
+print("Finished %s with %d tokens (in %d tests)." % (', '.join(tbmd.targetFiles),
  len(res),
- 

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Conversion using the ``futurize`` script, then manual review. For the sake of 
reviewer sanity, I've left all the difficult part to further separate commits.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55121



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

[PATCH] D55121: Make several Python scripts portable across Python2 and Python 3

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: ddunbar, djasper.
Herald added subscribers: cfe-commits, kadircet, arphaman, whisperity.

  Mostly:
  
  - portability of print through from __future__ import print_function
  - xrange -> range
  - range(...) -> list(range(...))
  - x.items() -> list(x.items())
  - map(f, l) -> list(map(f, l))
  - normalize shebang
  
  No extra dependency added.


Repository:
  rC Clang

https://reviews.llvm.org/D55121

Files:
  bindings/python/clang/cindex.py
  bindings/python/examples/cindex/cindex-dump.py
  bindings/python/tests/cindex/test_translation_unit.py
  docs/conf.py
  docs/tools/dump_format_style.py
  tools/clang-format/clang-format-diff.py
  tools/scan-build-py/libscanbuild/arguments.py
  tools/scan-view/share/Reporter.py
  utils/ABITest/ABITestGen.py
  utils/ABITest/Enumeration.py
  utils/CIndex/completion_logger_server.py
  utils/TestUtils/deep-stack.py
  utils/analyzer/SATestAdd.py
  utils/analyzer/SATestUpdateDiffs.py
  utils/analyzer/SumTimerInfo.py
  utils/check_cfc/obj_diff.py
  utils/check_cfc/setup.py
  utils/check_cfc/test_check_cfc.py
  utils/clangdiag.py
  utils/modfuzz.py
  utils/perf-training/perf-helper.py
  utils/token-delta.py
  www/builtins.py

Index: www/builtins.py
===
--- www/builtins.py
+++ www/builtins.py
@@ -151,7 +151,7 @@
   sys.stderr.write("%s:%d: x86 builtin %s used, too many replacements\n" % (fileinput.filename(), fileinput.filelineno(), builtin))
 
 for line in fileinput.input(inplace=1):
-  for builtin, repl in repl_map.iteritems():
+  for builtin, repl in repl_map.items():
 if builtin in line:
   line = line.replace(builtin, repl)
   report_repl(builtin, repl)
Index: utils/token-delta.py
===
--- utils/token-delta.py
+++ utils/token-delta.py
@@ -1,5 +1,6 @@
 #!/usr/bin/env python
 
+from __future__ import print_function
 import os
 import re
 import subprocess
@@ -37,7 +38,7 @@
 # O(N^2) case unless user requests it.
 if not force:
 if not self.getTestResult(changes):
-raise ValueError,'Initial test passed to delta fails.'
+raise ValueError('Initial test passed to delta fails.')
 
 # Check empty set first to quickly find poor test functions.
 if self.getTestResult(set()):
@@ -94,7 +95,7 @@
 
 ###
 
-class Token:
+class Token(object):
 def __init__(self, type, data, flags, file, line, column):
 self.type   = type
 self.data   = data
@@ -165,7 +166,7 @@
 byFile = self.writeFiles(changes, self.tempFiles)
 
 if self.log:
-print >>sys.stderr, 'TEST - ',
+print('TEST - ', end=' ', file=sys.stderr)
 if self.log > 1:
 for i,(file,_) in enumerate(self.tokenLists):
 indices = byFile[i]
@@ -184,8 +185,8 @@
 sys.stderr.write(str(byFile[i][-1]))
 sys.stderr.write('] ')
 else:
-print >>sys.stderr, ', '.join(['%s:%d tokens' % (file, len(byFile[i]))
-   for i,(file,_) in enumerate(self.tokenLists)]),
+print(', '.join(['%s:%d tokens' % (file, len(byFile[i]))
+   for i,(file,_) in enumerate(self.tokenLists)]), end=' ', file=sys.stderr)
 
 p = subprocess.Popen([self.testProgram] + self.tempFiles)
 res = p.wait() == 0
@@ -194,10 +195,10 @@
 self.writeFiles(changes, self.targetFiles)
 
 if self.log:
-print >>sys.stderr, '=> %s' % res
+print('=> %s' % res, file=sys.stderr)
 else:
 if res:
-print '\nSUCCESS (%d tokens)' % len(changes)
+print('\nSUCCESS (%d tokens)' % len(changes))
 else:
 sys.stderr.write('.')
 
@@ -209,7 +210,7 @@
   for j in range(len(tokens))])
 self.writeFiles(res, self.targetFiles)
 if not self.log:
-print >>sys.stderr
+print(file=sys.stderr)
 return res
 
 def tokenBasedMultiDelta(program, files, log):
@@ -218,15 +219,15 @@
   for file in files]
 
 numTokens = sum([len(tokens) for _,tokens in tokenLists])
-print "Delta on %s with %d tokens." % (', '.join(files), numTokens)
+print("Delta on %s with %d tokens." % (', '.join(files), numTokens))
 
 tbmd = TMBDDelta(program, tokenLists, log)
 
 res = tbmd.run()
 
-print "Finished %s with %d tokens (in %d tests)." % (', '.join(tbmd.targetFiles),
+print("Finished %s with %d tokens (in %d tests)." % (', '.join(tbmd.targetFiles),
  len(res),
- tbmd.numTests)
+