[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-14 Thread Rahman Lavaee via Phabricator via cfe-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7841e21c9849: Let -basic-block-sections=labels emit 
basicblock metadata in a new .bb_addr_map… (authored by rahmanl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-14 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 291607.
rahmanl added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad [[FOOCOMDAT_BEGIN]]
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
---

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-14 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 291595.
rahmanl added a comment.

- Merge branch 'master' into arcpatch-D85408


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad [[FOOCOMDAT_BEGIN]]
Index: llvm/lib/MC/MCObjectFileInfo.cpp

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-14 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 291589.
rahmanl added a comment.

- Remove the "labels" part of the clang test as the functionality is tested on 
LLVM tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad [[FOOCOMDAT_BEGIN]]
Index:

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-11 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: 
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:1
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections 
-basic-block-sections=labels | FileCheck %s
+

You can drop `-unknown-linux-gnu`. This is a generic ELF feature.



Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels.ll:2
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections 
-basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections 
-basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 

Delete `-check-prefix=CHECK`
It is the default


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-11 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added a comment.

In D85408#2266559 , @dblaikie wrote:

> In D85408#2262134 , @rahmanl wrote:
>
>> @efriedma Would you please chime in specially with respect to @MaskRay 's 
>> comment about the clang test involving assembly?
>
> I'll chime in, perhaps @efriedma will/can too: This patch has no changes to 
> Clang's code, so it should have no changes to Clang's tests, generally 
> speaking.
>
> Whatever codepath/behavior is being tested by that Clang test change should 
> be testable (& generally only tested) via LLVM's tools/tests, like llc, I 
> would expect? (I guess that's what the basic-block-sections-labels.ll test is 
> doing? Making the Clang test update redundant)
>
> The only reason I'd expect to see the assembly tested in Clang is if the flag 
> that enables this feature is an MCOptions (or other similar API level 
> feature) member, rather than part of LLVM IR proper. In that case the only 
> way to test that the flag has any effect (since we can't observe the 
> MCOptions struct state directly in a test) is to test for the MCOptions 
> effect on LLVM's output. Depending on the complexity, sometimes such tests 
> are just skipped entirely (leaving the setting of the MCOption untested) - I 
> think I did that for, for example, DWARF type units. Other times, an 
> end-to-end test is used to ensure the flag is working. But that's all the 
> test should do: test the flag is working, with some very basic/rudimentary 
> property of the feature being enabled (so pretty much /any/ change to the way 
> the feature works will still pass the test, but if the feature were turned 
> off entirely (the MCOptions flag stopped being set correctly), the test would 
> fail). Not test all the functionality of the feature that the flag enables - 
> all that testing should be down in LLVM.

Thanks for the clear explanation @dblaikie. In light of your comment,  I 
removed the complex assembly from the clang test, since the LLVM tests are 
already testing those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-11 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 291156.
rahmanl added a comment.

- Remove the complex assembly from the clang test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad [[FOOCOMDAT_BEGIN]]
Index: llvm/lib/MC/MCObjectFileInfo.cpp

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-10 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

In D85408#2262134 , @rahmanl wrote:

> @efriedma Would you please chime in specially with respect to @MaskRay 's 
> comment about the clang test involving assembly?

I'll chime in, perhaps @efriedma will/can too: This patch has no changes to 
Clang's code, so it should have no changes to Clang's tests, generally speaking.

Whatever codepath/behavior is being tested by that Clang test change should be 
testable (& generally only tested) via LLVM's tools/tests, like llc, I would 
expect? (I guess that's what the basic-block-sections-labels.ll test is doing? 
Making the Clang test update redundant)

The only reason I'd expect to see the assembly tested in Clang is if the flag 
that enables this feature is an MCOptions (or other similar API level feature) 
member, rather than part of LLVM IR proper. In that case the only way to test 
that the flag has any effect (since we can't observe the MCOptions struct state 
directly in a test) is to test for the MCOptions effect on LLVM's output. 
Depending on the complexity, sometimes such tests are just skipped entirely 
(leaving the setting of the MCOption untested) - I think I did that for, for 
example, DWARF type units. Other times, an end-to-end test is used to ensure 
the flag is working. But that's all the test should do: test the flag is 
working, with some very basic/rudimentary property of the feature being enabled 
(so pretty much /any/ change to the way the feature works will still pass the 
test, but if the feature were turned off entirely (the MCOptions flag stopped 
being set correctly), the test would fail). Not test all the functionality of 
the feature that the flag enables - all that testing should be down in LLVM.




Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1058
+  // Emit BB Information for each basic block in the funciton.
+  for (const auto  : MF) {
+const MCSymbol *MBBSymbol =

MaskRay wrote:
> auto -> MachineBasicBlock
FWIW, auto seems pretty fine to me here - looking only at for loops over MBB, a 
rough grep/count seems like there's certainly a lot of both (194 (non-auto) to 
168 (auto)) and I think it qualifies as "obvious" for LLVM backend code that an 
MBB variable would be of the type MachineBasicBlock. (bonus points for 
obviousness that MF is a MachineFunction)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added a comment.

@efriedma Would you please chime in specially with respect to @MaskRay 's 
comment about the clang test involving assembly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

I like the simplification:) However, I still feel puzzled by the large chunk of 
assembly test in `clang/test/CodeGen/basic-block-sections.c`. I hope another 
reviewer or the code owner can say that this is ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added a comment.

Thanks for the review @MaskRay. Is this ready to land now?




Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

MaskRay wrote:
> rahmanl wrote:
> > MaskRay wrote:
> > > BBAddrMapSection is always non-null. Delete the if.
> > I believe we return null for non-ELF environment (Please refer to 
> > MCObjectFileInfo::getBBAddrMapSection).
> In that case emitBBAddrMapSection will not be called?
Fair enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-08 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 290511.
rahmanl marked an inline comment as done.
rahmanl added a comment.

- Merge remote-tracking branch 'origin/master' into arcpatch-D85408


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+; CHECK-LABEL:	_Z4fooTIiET_v:
+; CHECK-NEXT:	[[FOOCOMDAT_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}
+; CHECK-NEXT:		.quad

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

rahmanl wrote:
> MaskRay wrote:
> > BBAddrMapSection is always non-null. Delete the if.
> I believe we return null for non-ELF environment (Please refer to 
> MCObjectFileInfo::getBBAddrMapSection).
In that case emitBBAddrMapSection will not be called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

MaskRay wrote:
> BBAddrMapSection is always non-null. Delete the if.
I believe we return null for non-ELF environment (Please refer to 
MCObjectFileInfo::getBBAddrMapSection).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-03 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 289765.
rahmanl marked 5 inline comments as done.
rahmanl added a comment.

- Address +MaskRay's comments:
- Change the check prefix simply to "CHECK" for basic-block-sections-labels.ll
- Change the triple to x86_64 for this test.
- nits.
- Remove the "-LABEL" feature from basic-block-sections-labels.ll and use 
precise BB indices.
- Remove the assertion in emitBBAddrMapSection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64 -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=CHECK
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; CHECK-LABEL:	_Z3bazb:
+; CHECK-LABEL:	.Lfunc_begin0:
+; CHECK-LABEL:	.LBB_END0_0:
+; CHECK-LABEL:	.LBB0_1:
+; CHECK-LABEL:	.LBB_END0_1:
+; CHECK-LABEL:	.LBB0_2:
+; CHECK-LABEL:	.LBB_END0_2:
+; CHECK-LABEL:	.LBB0_3:
+; CHECK-LABEL:	.LBB_END0_3:
+; CHECK-LABEL:	.Lfunc_end0:
+
+; CHECK:	.section	.bb_addr_map,"o",@progbits,.text
+; CHECK-NEXT:	.quad	.Lfunc_begin0
+; CHECK-NEXT:	.byte	4
+; CHECK-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_0-.Lfunc_begin0
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_1-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_1-.LBB0_1
+; CHECK-NEXT:	.byte	0
+; CHECK-NEXT:	.uleb128 .LBB0_2-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_2-.LBB0_2
+; CHECK-NEXT:	.byte	1
+; CHECK-NEXT:	.uleb128 .LBB0_3-.Lfunc_begin0
+; CHECK-NEXT:	.uleb128 .LBB_END0_3-.LBB0_3
+; CHECK-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:		.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:		.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:		.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:		.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:		.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:		.section

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Thanks, I have followed the flow. The logic looks good. But I have got some 
nits and a question about the `clang/test/` assembly test.




Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1033
+///  * 2nd bit: set if it is a block ending with a tail call.
+///  * 3rd bit: set if it is an exception handling (EH) pad.
+/// The remaining bits are zero.

1st 2nd 3rd may be confusing. You can just write: 1<<0, 1<<1, and 1<<2. 
Actually, you may just say: use 3 bits to encode information for more precise 
profile, (1) (2) (3).

The first two sentences can thus be simplified.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1047
+  getObjFileLowering().getBBAddrMapSection(*MF.getSection());
+  if (!BBAddrMapSection)
+return;

BBAddrMapSection is always non-null. Delete the if.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1058
+  // Emit BB Information for each basic block in the funciton.
+  for (const auto  : MF) {
+const MCSymbol *MBBSymbol =

auto -> MachineBasicBlock



Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels.ll:2
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections 
-basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections 
-basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 

`LINUX-LABELS` is probably not a good CHECK prefix. The feature is generic and 
not specific to linux (i.e. you could use `-mtriple=x86_64`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

In D85408#2253073 , @tmsriram wrote:

> In D85408#2253055 , @MaskRay wrote:
>
>> I am still reading the patch, but I have noticed one thing worth discussing. 
>> `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of 
>> the memory image). An absolute relocation type (`.quad
>> .Lfunc_begin0`) works but the value is a link-time address, not taking 
>> account of the load base (PIE/shared object)). (If .bb_addr_map has the 
>> SHF_ALLOC flag, linkers will report a text relocation) How do you intend to 
>> use `.bb_addr_map` at runtime?
>
> bb_addr_map is intentionally not loaded as it is never meant to be used at 
> run-time. bb_addr_map will be later be used in conjunction with a hardware 
> profile of the binary to associate executed basic block addresses to machine 
> basic blocks.  The hardware profile of the binary has enough info to 
> determine the load base address with PIE, this is pretty straight forward and 
> multiple strategies exist.  What exactly is the concern here?

Thanks for the explanation! I guessed that non-SHF_ALLOC is intended but hoped 
that it can confirmed (and you did!). This sections works similar to 
`.stack_sizes` emitted by clang -fstack-size-section.

This is important because for SHF_ALLOC, SHF_WRITE is also needed to avoid text 
relocations. I am still reading the patch to ensure that it will work as 
intended.




Comment at: 
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:21
+; CHECK:   .section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL: _Z3foov:
+; CHECK-NEXT:  [[FOO_BEGIN:.Lfunc_begin[0-9]+]]:

Consider align the content. Since `CHECK-LABEL:` is the longest. 

```
; CHECK:.section .text._Z3foov,"ax",@progbits
; CHECK-LABEL:  _Z3foov:
; CHECK-NEXT: [[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
```

Non-labels have additional indentation to make it clear they are bodies covered 
by labels.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Han Shen via Phabricator via cfe-commits 
shenhan added a comment.

In D85408#2253055 , @MaskRay wrote:

> I am still reading the patch, but I have noticed one thing worth discussing. 
> `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the 
> memory image). An absolute relocation type (`.quad .Lfunc_begin0`) works but 
> the value is a link-time address, not taking account of the load base 
> (PIE/shared object)). (If .bb_addr_map has the SHF_ALLOC flag, linkers will 
> report a text relocation) How do you intend to use `.bb_addr_map` at runtime?

Regarding "not taking account of the load base (PIE/shared object)" - the 
profile conversion tool uses addresses in .bb_addr_map section and MMAP entries 
in perf.data file to construct runtime addresses, so these runtime addresses 
can be matched with the profile addresses recorded in perf.data file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Sriraman Tallam via Phabricator via cfe-commits 
tmsriram added a comment.

In D85408#2253055 , @MaskRay wrote:

> I am still reading the patch, but I have noticed one thing worth discussing. 
> `.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the 
> memory image). An absolute relocation type (`.quad .Lfunc_begin0`) works but 
> the value is a link-time address, not taking account of the load base 
> (PIE/shared object)). (If .bb_addr_map has the SHF_ALLOC flag, linkers will 
> report a text relocation) How do you intend to use `.bb_addr_map` at runtime?

bb_addr_map is intentionally not loaded as it is never meant to be used at 
run-time. bb_addr_map will be later be used in conjunction with a hardware 
profile of the binary to associate executed basic block addresses to machine 
basic blocks.  The hardware profile of the binary has enough info to determine 
the load base address with PIE, this is pretty straight forward and multiple 
strategies exist.  What exactly is the concern here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

I am still reading the patch, but I have noticed one thing worth discussing. 
`.bb_addr_map` is a non-SHF_ALLOC section (meaning that it is not part of the 
memory image). An absolute relocation type (`.quad   .Lfunc_begin0`) works but 
the value is a link-time address, not taking account of the load base 
(PIE/shared object)). How do you intend to use `.bb_addr_map` at runtime?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a subscriber: rsmith.
MaskRay added inline comments.



Comment at: clang/test/CodeGen/basic-block-sections.c:31
 // BB_LABELS: world:
-// BB_LABELS: a.BB.world:
-// BB_LABELS: aa.BB.world:
-// BB_LABELS: a.BB.another:
+// BB_LABELS: .Lfunc_begin0:
+// BB_LABELS: .LBB_END0_0:

Assembly tests are not common in clang/test but @rsmith LGTMed D68049 so I 
think it should be fine. Now this file tests more machine level stuff. Should 
the file switch to LLVM IR?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added a comment.

Thanks a lot for the comments @MaskRay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-09-02 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 289498.
rahmanl marked 5 inline comments as done.
rahmanl added a comment.

- Address @MaskRay's comments.
- Rebase with upstream.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.Lfunc_begin[0-9]+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:	.section

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-31 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:78
 } else {
+  auto Prefix = Ctx.getAsmInfo()->getPrivateLabelPrefix();
   CachedMCSymbol = Ctx.getOrCreateSymbol(Twine(Prefix) + "BB" +

Expand `auto`

https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable



Comment at: 
llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll:33
+; CHECK-LABEL: _Z4fooTIiET_v:
+; CHECK-NEXT:  [[FOOCOMDAT_BEGIN:.+]]:
+; CHECK:   .section 
.bb_addr_map,"Go",@progbits,_Z4fooTIiET_v,comdat,.text._Z4fooTIiET_v{{$}}

Can you use a more specific pattern than `.+`? It is difficult to know which 
pattern it uses (a larger issue is that it matches spaces and possibly 
leverages a subtle property of FileCheck)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-31 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/docs/UsersManual.rst:1703
 
-  Controls whether Clang emits a label for each basic block.  Further, with
-  values "all" and "list=arg", each basic block or a subset of basic blocks
-  can be placed in its own unique section.
+  Controls how Clang emits text sections for basic blocks. With values "all"
+  and "list=arg", each basic block or a subset of basic blocks can be placed in

Use backquotes like below



Comment at: clang/test/CodeGen/basic-block-sections.c:3
 
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -o - < %s | FileCheck %s 
--check-prefix=PLAIN
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -S -fbasic-block-sections=all 
-fbasic-block-sections=none -o - < %s | FileCheck %s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -o - < %s | FileCheck 
%s --check-prefix=PLAIN
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S 
-fbasic-block-sections=all -fbasic-block-sections=none -o - < %s | FileCheck %s 
--check-prefix=PLAIN

The triple change is unneeded. You can also just use `-triple x86_64` because 
the behavior applies to generic ELF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-28 Thread Snehasish Kumar via Phabricator via cfe-commits 
snehasish accepted this revision.
snehasish added a comment.
This revision is now accepted and ready to land.

LGTM. Please wait a bit for additional comments from others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-28 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl marked an inline comment as done.
rahmanl added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1030
+/// recursive return edges vs. indirect branches. The format of the metadata is
+/// described as follows: 1st bit (LSB): set if this is a return block (return
+/// or tail call). 2nd bit: set if this is a block ending with a tail call. 3rd

snehasish wrote:
> Did you intend to change the formatting of the comment? I think the list 
> items were on separate lines.
My bad. Didn't fully check the lint result.



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:58
 
+unsigned MachineBasicBlock::getBBAddrMapMetadata() const {
+  const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();

snehasish wrote:
> rahmanl wrote:
> > snehasish wrote:
> > > I think this method only relies on already public methods of 
> > > MachineBasicBlock. It would be cleaner to move this from here to a static 
> > > helper function in AsmPrinter.cpp. This way we don't add yet another 
> > > method to the MachineBasicBlock class and keeps the relevant metadata 
> > > generation logic close to where it is used.
> > > 
> > > Feel free to ignore if you plan on calling this method elsewhere apart 
> > > from AsmPrinter.cpp but I can't think of a reason to do so.
> > Thanks for the suggestion. It makes the code much more readable since I 
> > just place that function above emitBBAddrMapSection.
> > I can even make it a lambda function within emitBBAddrMapSection. WDYT?
> I have a slight preference for the static method vs inline lambda since the 
> comment describing the metadata can be placed above the static method.
Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-28 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 288694.
rahmanl added a comment.

- Move getBBAddrMetadata to AsmPrinter as a static function + nits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:	.section .text._Z4fooTIiET_v,"axG",@progbits,_Z4fooTIiET_v,comdat
+;

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Snehasish Kumar via Phabricator via cfe-commits 
snehasish added inline comments.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1030
+/// recursive return edges vs. indirect branches. The format of the metadata is
+/// described as follows: 1st bit (LSB): set if this is a return block (return
+/// or tail call). 2nd bit: set if this is a block ending with a tail call. 3rd

Did you intend to change the formatting of the comment? I think the list items 
were on separate lines.



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:58
 
+unsigned MachineBasicBlock::getBBAddrMapMetadata() const {
+  const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();

rahmanl wrote:
> snehasish wrote:
> > I think this method only relies on already public methods of 
> > MachineBasicBlock. It would be cleaner to move this from here to a static 
> > helper function in AsmPrinter.cpp. This way we don't add yet another method 
> > to the MachineBasicBlock class and keeps the relevant metadata generation 
> > logic close to where it is used.
> > 
> > Feel free to ignore if you plan on calling this method elsewhere apart from 
> > AsmPrinter.cpp but I can't think of a reason to do so.
> Thanks for the suggestion. It makes the code much more readable since I just 
> place that function above emitBBAddrMapSection.
> I can even make it a lambda function within emitBBAddrMapSection. WDYT?
I have a slight preference for the static method vs inline lambda since the 
comment describing the metadata can be placed above the static method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl added inline comments.



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:58
 
+unsigned MachineBasicBlock::getBBAddrMapMetadata() const {
+  const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();

snehasish wrote:
> I think this method only relies on already public methods of 
> MachineBasicBlock. It would be cleaner to move this from here to a static 
> helper function in AsmPrinter.cpp. This way we don't add yet another method 
> to the MachineBasicBlock class and keeps the relevant metadata generation 
> logic close to where it is used.
> 
> Feel free to ignore if you plan on calling this method elsewhere apart from 
> AsmPrinter.cpp but I can't think of a reason to do so.
Thanks for the suggestion. It makes the code much more readable since I just 
place that function above emitBBAddrMapSection.
I can even make it a lambda function within emitBBAddrMapSection. WDYT?



Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels.ll:5-9
   %2 = alloca i8, align 1
   %3 = zext i1 %0 to i8
   store i8 %3, i8* %2, align 1
   %4 = load i8, i8* %2, align 1
   %5 = trunc i8 %4 to i1

snehasish wrote:
> I think this can be removed and the branch on L10 can directly use the param 
> like so:
> 
> ```
> define void @_Z3bazb(i1 zeroext %0) personality i32 (...)* 
> @__gxx_personality_v0 {
>   br i1 %0, label %6, label %11
>   
> }
> ```
> 
> You will need to adjust the respective virtual register names since %2-%5 is 
> no longer used.
Great suggestion. Thanks. This was copied over from other basic-block-sections 
tests, so maybe they can be updated too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 288158.
rahmanl marked 4 inline comments as done.
rahmanl added a comment.

Address Snehasish's comments.

- Move getBBAddrMetadata to AsmPrinter as a static function in AsmPrinter
- Remove unnecessary IR instructions in basic-block-sections-labels.ll


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,24 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
-  %2 = alloca i8, align 1
-  %3 = zext i1 %0 to i8
-  store i8 %3, i8* %2, align 1
-  %4 = load i8, i8* %2, align 1
-  %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
+  br i1 %0, label %2, label %7
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+2:
+  %3 = invoke i32 @_Z3barv()
+  to label %7 unwind label %5
+  br label %9
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+5:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %9
 
-10:   ; preds = %8, %6
+7:
+  %8 = call i32 @_Z3foov()
+  br label %9
+
+9:
   ret void
 }
 
@@ -25,9 +26,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Snehasish Kumar via Phabricator via cfe-commits 
snehasish added inline comments.



Comment at: clang/docs/UsersManual.rst:1706
+  its own unique section. With the "labels" value, normal sections are emitted,
+  but a ".bb_addr_map" section is emitted which would include address offsets
+  for each basic block in the program, relative to the function address.

nit: s/would include/includes/



Comment at: clang/docs/UsersManual.rst:1707
+  but a ".bb_addr_map" section is emitted which would include address offsets
+  for each basic block in the program, relative to the function address.
 

nit: s/relative to the/relative to the parent/



Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:58
 
+unsigned MachineBasicBlock::getBBAddrMapMetadata() const {
+  const TargetInstrInfo *TII = getParent()->getSubtarget().getInstrInfo();

I think this method only relies on already public methods of MachineBasicBlock. 
It would be cleaner to move this from here to a static helper function in 
AsmPrinter.cpp. This way we don't add yet another method to the 
MachineBasicBlock class and keeps the relevant metadata generation logic close 
to where it is used.

Feel free to ignore if you plan on calling this method elsewhere apart from 
AsmPrinter.cpp but I can't think of a reason to do so.



Comment at: llvm/test/CodeGen/X86/basic-block-sections-labels.ll:5-9
   %2 = alloca i8, align 1
   %3 = zext i1 %0 to i8
   store i8 %3, i8* %2, align 1
   %4 = load i8, i8* %2, align 1
   %5 = trunc i8 %4 to i1

I think this can be removed and the branch on L10 can directly use the param 
like so:

```
define void @_Z3bazb(i1 zeroext %0) personality i32 (...)* 
@__gxx_personality_v0 {
  br i1 %0, label %6, label %11
  
}
```

You will need to adjust the respective virtual register names since %2-%5 is no 
longer used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

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

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 288105.
rahmanl added a comment.

Fix failing clang test which used to check for the old ".bb_info" name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/docs/UsersManual.rst
  clang/test/CodeGen/basic-block-sections.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/MIRParser/MIRParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
  llvm/test/CodeGen/X86/basic-block-sections-labels.ll

Index: llvm/test/CodeGen/X86/basic-block-sections-labels.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections-labels.ll
+++ llvm/test/CodeGen/X86/basic-block-sections-labels.ll
@@ -1,23 +1,29 @@
 ; Check the basic block sections labels option
-; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s -check-prefix=LINUX-LABELS
 
-define void @_Z3bazb(i1 zeroext) {
+define void @_Z3bazb(i1 zeroext) personality i32 (...)* @__gxx_personality_v0 {
   %2 = alloca i8, align 1
   %3 = zext i1 %0 to i8
   store i8 %3, i8* %2, align 1
   %4 = load i8, i8* %2, align 1
   %5 = trunc i8 %4 to i1
-  br i1 %5, label %6, label %8
+  br i1 %5, label %6, label %11
 
-6:; preds = %1
-  %7 = call i32 @_Z3barv()
-  br label %10
+6:
+  %7 = invoke i32 @_Z3barv()
+  to label %11 unwind label %9
+  br label %13
 
-8:; preds = %1
-  %9 = call i32 @_Z3foov()
-  br label %10
+9:
+  landingpad { i8*, i32 }
+  catch i8* null
+  br label %13
 
-10:   ; preds = %8, %6
+11:
+  %12 = call i32 @_Z3foov()
+  br label %13
+
+13:
   ret void
 }
 
@@ -25,9 +31,31 @@
 
 declare i32 @_Z3foov() #1
 
-; LINUX-LABELS: .section
-; LINUX-LABELS: _Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: r.BB._Z3bazb:
-; LINUX-LABELS-NOT: .section
-; LINUX-LABELS: rr.BB._Z3bazb:
+declare i32 @__gxx_personality_v0(...)
+
+; LINUX-LABELS-LABEL:	_Z3bazb:
+; LINUX-LABELS:		.Lfunc_begin0:
+; LINUX-LABELS:		.LBB_END0_[[L1:[0-9]+]]:
+; LINUX-LABELS:		.LBB0_[[L2:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L2]]:
+; LINUX-LABELS:		.LBB0_[[L3:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L3]]:
+; LINUX-LABELS:		.LBB0_[[L4:[0-9]+]]:
+; LINUX-LABELS:		.LBB_END0_[[L4]]:
+; LINUX-LABELS:		.Lfunc_end0:
+
+; LINUX-LABELS:		.section	.bb_addr_map,"o",@progbits,.text
+; LINUX-LABELS-NEXT:	.quad	.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	4
+; LINUX-LABELS-NEXT:	.uleb128 .Lfunc_begin0-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L1]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L2]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L2]]-.LBB0_[[L2]]
+; LINUX-LABELS-NEXT:	.byte	0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L3]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L3]]-.LBB0_[[L3]]
+; LINUX-LABELS-NEXT:	.byte	1
+; LINUX-LABELS-NEXT:	.uleb128 .LBB0_[[L4]]-.Lfunc_begin0
+; LINUX-LABELS-NEXT:	.uleb128 .LBB_END0_[[L4]]-.LBB0_[[L4]]
+; LINUX-LABELS-NEXT:	.byte	5
Index: llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/basic-block-sections-labels-functions-sections.ll
@@ -0,0 +1,35 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -function-sections -basic-block-sections=labels | FileCheck %s
+
+$_Z4fooTIiET_v = comdat any
+
+define dso_local i32 @_Z3barv() {
+  ret i32 0
+}
+;; Check we add SHF_LINK_ORDER for .bb_addr_map and link it with the corresponding .text sections.
+; CHECK:	.section .text._Z3barv,"ax",@progbits
+; CHECK-LABEL:	_Z3barv:
+; CHECK-NEXT:	[[BAR_BEGIN:.+]]:
+; CHECK:	.section .bb_addr_map,"o",@progbits,.text._Z3barv{{$}}
+; CHECK-NEXT:	.quad [[BAR_BEGIN]]
+
+
+define dso_local i32 @_Z3foov() {
+  %1 = call i32 @_Z4fooTIiET_v()
+  ret i32 %1
+}
+; CHECK:	.section .text._Z3foov,"ax",@progbits
+; CHECK-LABEL:	_Z3foov:
+; CHECK-NEXT:	[[FOO_BEGIN:.+]]:
+; CHECK:	.section  .bb_addr_map,"o",@progbits,.text._Z3foov{{$}}
+; CHECK-NEXT:	.quad [[FOO_BEGIN]]
+
+
+define linkonce_odr dso_local i32 @_Z4fooTIiET_v() comdat {
+  ret i32 0
+}
+;; Check we add .bb_addr_map section to a COMDAT group with the corresponding .text section if such a COMDAT exists.
+; CHECK:	.section

[PATCH] D85408: Let -basic-block-sections=labels emit basicblock metadata in a new .bb_addr_map section, instead of emitting special unary-encoded symbols.

2020-08-26 Thread Rahman Lavaee via Phabricator via cfe-commits 
rahmanl updated this revision to Diff 288104.
rahmanl edited the summary of this revision.
rahmanl added a comment.

Fix failing clang test which used to check for the old ".bb_info" name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85408

Files:
  clang/test/CodeGen/basic-block-sections.c


Index: clang/test/CodeGen/basic-block-sections.c
===
--- clang/test/CodeGen/basic-block-sections.c
+++ clang/test/CodeGen/basic-block-sections.c
@@ -40,7 +40,7 @@
 // BB_LABELS: .LBB_END0_5:
 // BB_LABELS: .Lfunc_end0:
 //
-// BB_LABELS:   .section  .bb_info,"o",@progbits,.text
+// BB_LABELS:   .section  .bb_addr_map,"o",@progbits,.text
 // BB_LABELS-NEXT:  .quad  .Lfunc_begin0
 // BB_LABELS-NEXT:  .byte  6
 // BB_LABELS-NEXT:  .uleb128 .Lfunc_begin0-.Lfunc_begin0
@@ -73,7 +73,7 @@
 // BB_LABELS: .LBB_END1_3:
 // BB_LABELS: .Lfunc_end1:
 //
-// BB_LABELS:   .section  .bb_info,"o",@progbits,.text
+// BB_LABELS:   .section  .bb_addr_map,"o",@progbits,.text
 // BB_LABELS-NEXT:  .quad  .Lfunc_begin1
 // BB_LABELS-NEXT:  .byte  4
 // BB_LABELS-NEXT:  .uleb128 .Lfunc_begin1-.Lfunc_begin1


Index: clang/test/CodeGen/basic-block-sections.c
===
--- clang/test/CodeGen/basic-block-sections.c
+++ clang/test/CodeGen/basic-block-sections.c
@@ -40,7 +40,7 @@
 // BB_LABELS: .LBB_END0_5:
 // BB_LABELS: .Lfunc_end0:
 //
-// BB_LABELS:   .section  .bb_info,"o",@progbits,.text
+// BB_LABELS:   .section  .bb_addr_map,"o",@progbits,.text
 // BB_LABELS-NEXT:  .quad  .Lfunc_begin0
 // BB_LABELS-NEXT:  .byte  6
 // BB_LABELS-NEXT:  .uleb128 .Lfunc_begin0-.Lfunc_begin0
@@ -73,7 +73,7 @@
 // BB_LABELS: .LBB_END1_3:
 // BB_LABELS: .Lfunc_end1:
 //
-// BB_LABELS:   .section  .bb_info,"o",@progbits,.text
+// BB_LABELS:   .section  .bb_addr_map,"o",@progbits,.text
 // BB_LABELS-NEXT:  .quad  .Lfunc_begin1
 // BB_LABELS-NEXT:  .byte  4
 // BB_LABELS-NEXT:  .uleb128 .Lfunc_begin1-.Lfunc_begin1
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits