[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

Thanks MaskRay for your review comments!




Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1752
+  else {
+UniqueID = NextUniqueID++;
+COMDATSymName = MBB.getParent()->getName();

MaskRay wrote:
> I think `UniqueID = NextUniqueID++;` is not needed.
I added comment (line 1752) to explain why add this line, this line referred to 
ELF implementation, please refer to 
https://github.com/llvm/llvm-project/blob/cb2a2ba8d64da5be3fac0ea90e406270e8a615bd/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L993



Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1756
+
+  // TODO: construct cold section in the case of section ID of MBB is
+  // MBBSectionID::ColdSectionID

MaskRay wrote:
> Can you fix the TODO in this patch?
It's indeed TODO. MMBSectionID::ColdSectionID will be handled in MFS on Windows 
COFF patch. I plan to re-base my previous MFS on Windows COFF patch (D95209) 
after this patch. I also explained this TODO in Summary.



Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1786
+  // COFF linker will not properly handle comdats otherwise.
+  if (getContext().getTargetTriple().isWindowsGNUEnvironment()) {
+Name += '$';

MaskRay wrote:
> Is `isOSBinFormatCOFF()` more appropriate? Can windows-gnu use the 
> optimization?
Windows-gnu can use the optimization. In the case of Windows-msvc, 
isOSBinFormatCOFF() is true, but the section name should not include 
COMDATSymName suffix.
This part referred to existing code 
https://github.com/llvm/llvm-project/blob/cb2a2ba8d64da5be3fac0ea90e406270e8a615bd/llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L1663
Thanks for this Windows-gnu comment! I reviewed Window-gnu related 
implementation and fixed Windows-gnu problem of 
TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 367407.
TaoPan added a comment.

Fix Windows gnu section name and test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",one_only,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]],"xr",one_only,[[SECTION_LABEL_1]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]],"xr",one_only,[[SECTION_LABEL_2]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text$[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]],"xr"
+; WINDOWS-GNU-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-SECTIONS: .section.text$[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]],"xr"
+; WINDOWS-GNU-SECTIONS: [[SECTION_LABEL_2]]:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1736,6 +1736,75 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function , const MachineBasicBlock ,
+const TargetMachine ) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+
+  // Use a unique name, or a unique ID for the section.
+  StringRef 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

@rnk Thanks for your review comments! Could you please help to review my reply 
and new modification?
@MaskRay Could you please also help to review?




Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1712
+  COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+  SectionKind::getText(), COMDATSymName,
+  COFF::IMAGE_COMDAT_SELECT_NODUPLICATES, UniqueID);

rnk wrote:
> TaoPan wrote:
> > TaoPan wrote:
> > > TaoPan wrote:
> > > > mstorsjo wrote:
> > > > > rnk wrote:
> > > > > > mstorsjo wrote:
> > > > > > > TaoPan wrote:
> > > > > > > > rnk wrote:
> > > > > > > > > tmsriram wrote:
> > > > > > > > > > MaskRay wrote:
> > > > > > > > > > > TaoPan wrote:
> > > > > > > > > > > > tmsriram wrote:
> > > > > > > > > > > > > COMDATSymName can be folded to be equal to 
> > > > > > > > > > > > > MBB.getSymbol()->getName() here?  Plus, you are not 
> > > > > > > > > > > > > preserving the .text.hot prefix that the original 
> > > > > > > > > > > > > function might get here.  Is this future work?  If 
> > > > > > > > > > > > > the original function is named .text.hot.foo, the 
> > > > > > > > > > > > > basic block will still be named .text.foo.__part.1 
> > > > > > > > > > > > > which is not right.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Plus, what about exception handling sections like 
> > > > > > > > > > > > > ".eh.*"?
> > > > > > > > > > > > Thanks! I'll redesign section name and comdat symbol 
> > > > > > > > > > > > name.
> > > > > > > > > > > > The text section with prefix "hot" and "unlikely" won't 
> > > > > > > > > > > > be constructed here, I added COFF text section prefix 
> > > > > > > > > > > > "hot" and "unlikely" in D92073. In ELF override 
> > > > > > > > > > > > function, also not handling text section with prefix 
> > > > > > > > > > > > "hot" and "unlikely".
> > > > > > > > > > > > The text section with prefix "split" will be 
> > > > > > > > > > > > constructed here, I plan to add related code in MFS 
> > > > > > > > > > > > COFF patch.
> > > > > > > > > > > > Also, exception handling section is future work that 
> > > > > > > > > > > > support basic block sections Windows COFF exception 
> > > > > > > > > > > > handling.
> > > > > > > > > > > This is complex. PE-COFF has multiple COMDAT seletion 
> > > > > > > > > > > kinds. I want to see a holistic plan how every component 
> > > > > > > > > > > is going to be implemented.
> > > > > > > > > > The basic block should just mimic the COMDAT type of its 
> > > > > > > > > > containing function, is there a reason to do anything more 
> > > > > > > > > > with it here?
> > > > > > > > > After thinking about it a bit, I think the entry block should 
> > > > > > > > > use the regular selection kind, and all of the auxilliary MBB 
> > > > > > > > > sections should use IMAGE_COMDAT_SELECT_ASSOCIATIVE. They 
> > > > > > > > > should be associated with the main function symbol, unless 
> > > > > > > > > the function itself is associated with something else, in 
> > > > > > > > > which case the BBs use that symbol.
> > > > > > > > > 
> > > > > > > > > This will ensure that if the main function section prevails, 
> > > > > > > > > they are included, and if it does not prevail, they are 
> > > > > > > > > discarded. Does that make sense?
> > > > > > > > Thanks! I think set entry block sections as regular 
> > > > > > > > IMAGE_COMDAT_SELECT_NODUPLICATES and set the auxilliary MBB 
> > > > > > > > sections as IMAGE_COMDAT_SELECT_ASSOCIATIVE is fine. I changed.
> > > > > > > @rnk - I'm not quite familiar with the concrete implications of 
> > > > > > > this work here, but would this need to be mapped to the GNU style 
> > > > > > > comdats, for compatibility with ld.bfd for mingw targets? (LLD 
> > > > > > > itself works fine with proper associative comdats though.)
> > > > > > I think basic block sections are kind of in the same category as 
> > > > > > LTO: it's a very sophisticated optimization feature, and it's 
> > > > > > probably OK if it's LLD-only. I think it also requires special 
> > > > > > linker support that might make it LLD-only, but I've forgotten the 
> > > > > > details.
> > > > > > 
> > > > > > It also has potentially very high object file size overhead, and it 
> > > > > > will be important to do everything possible to minimize that object 
> > > > > > file size overhead. Long gnu-style section names for every basic 
> > > > > > block section are likely to make the feature unusably slow, so 
> > > > > > maybe it's worth removing these "if mingw" conditionals. We can 
> > > > > > leave behind a comment by the use of 
> > > > > > IMAGE_COMDAT_SELECT_ASSOCIATIVE indicating that gnu-style section 
> > > > > > names are not needed because the feature is only designed to work 
> > > > > > with LLD.
> > > > > Thanks for the clarification! Leaving the feature as LLD-only (or in 
> > > > > other terms, requiring a conforming COMDAT implementation) sounds 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 364013.
TaoPan added a comment.

Change selection of entry block text section from IMAGE_COMDAT_SELECT_ANY to 
return value of getSelectionForCOFF()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",one_only,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.1:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.2:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1733,6 +1733,65 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function , const MachineBasicBlock ,
+const TargetMachine ) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+  StringRef COMDATSymName;
+  if (TM.getUniqueBasicBlockSectionNames())
+COMDATSymName = MBB.getSymbol()->getName();
+  else {
+UniqueID = 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

@rnk could you please have a review of IMAGE_COMDAT_SELECT_XXX modification?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added inline comments.



Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1712
+  COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+  SectionKind::getText(), COMDATSymName,
+  COFF::IMAGE_COMDAT_SELECT_NODUPLICATES, UniqueID);

TaoPan wrote:
> TaoPan wrote:
> > mstorsjo wrote:
> > > rnk wrote:
> > > > mstorsjo wrote:
> > > > > TaoPan wrote:
> > > > > > rnk wrote:
> > > > > > > tmsriram wrote:
> > > > > > > > MaskRay wrote:
> > > > > > > > > TaoPan wrote:
> > > > > > > > > > tmsriram wrote:
> > > > > > > > > > > COMDATSymName can be folded to be equal to 
> > > > > > > > > > > MBB.getSymbol()->getName() here?  Plus, you are not 
> > > > > > > > > > > preserving the .text.hot prefix that the original 
> > > > > > > > > > > function might get here.  Is this future work?  If the 
> > > > > > > > > > > original function is named .text.hot.foo, the basic block 
> > > > > > > > > > > will still be named .text.foo.__part.1 which is not right.
> > > > > > > > > > > 
> > > > > > > > > > > Plus, what about exception handling sections like ".eh.*"?
> > > > > > > > > > Thanks! I'll redesign section name and comdat symbol name.
> > > > > > > > > > The text section with prefix "hot" and "unlikely" won't be 
> > > > > > > > > > constructed here, I added COFF text section prefix "hot" 
> > > > > > > > > > and "unlikely" in D92073. In ELF override function, also 
> > > > > > > > > > not handling text section with prefix "hot" and "unlikely".
> > > > > > > > > > The text section with prefix "split" will be constructed 
> > > > > > > > > > here, I plan to add related code in MFS COFF patch.
> > > > > > > > > > Also, exception handling section is future work that 
> > > > > > > > > > support basic block sections Windows COFF exception 
> > > > > > > > > > handling.
> > > > > > > > > This is complex. PE-COFF has multiple COMDAT seletion kinds. 
> > > > > > > > > I want to see a holistic plan how every component is going to 
> > > > > > > > > be implemented.
> > > > > > > > The basic block should just mimic the COMDAT type of its 
> > > > > > > > containing function, is there a reason to do anything more with 
> > > > > > > > it here?
> > > > > > > After thinking about it a bit, I think the entry block should use 
> > > > > > > the regular selection kind, and all of the auxilliary MBB 
> > > > > > > sections should use IMAGE_COMDAT_SELECT_ASSOCIATIVE. They should 
> > > > > > > be associated with the main function symbol, unless the function 
> > > > > > > itself is associated with something else, in which case the BBs 
> > > > > > > use that symbol.
> > > > > > > 
> > > > > > > This will ensure that if the main function section prevails, they 
> > > > > > > are included, and if it does not prevail, they are discarded. 
> > > > > > > Does that make sense?
> > > > > > Thanks! I think set entry block sections as regular 
> > > > > > IMAGE_COMDAT_SELECT_NODUPLICATES and set the auxilliary MBB 
> > > > > > sections as IMAGE_COMDAT_SELECT_ASSOCIATIVE is fine. I changed.
> > > > > @rnk - I'm not quite familiar with the concrete implications of this 
> > > > > work here, but would this need to be mapped to the GNU style comdats, 
> > > > > for compatibility with ld.bfd for mingw targets? (LLD itself works 
> > > > > fine with proper associative comdats though.)
> > > > I think basic block sections are kind of in the same category as LTO: 
> > > > it's a very sophisticated optimization feature, and it's probably OK if 
> > > > it's LLD-only. I think it also requires special linker support that 
> > > > might make it LLD-only, but I've forgotten the details.
> > > > 
> > > > It also has potentially very high object file size overhead, and it 
> > > > will be important to do everything possible to minimize that object 
> > > > file size overhead. Long gnu-style section names for every basic block 
> > > > section are likely to make the feature unusably slow, so maybe it's 
> > > > worth removing these "if mingw" conditionals. We can leave behind a 
> > > > comment by the use of IMAGE_COMDAT_SELECT_ASSOCIATIVE indicating that 
> > > > gnu-style section names are not needed because the feature is only 
> > > > designed to work with LLD.
> > > Thanks for the clarification! Leaving the feature as LLD-only (or in 
> > > other terms, requiring a conforming COMDAT implementation) sounds good to 
> > > me.
> > Thanks for discussion between @mstorsjo and @rnk !
> > I removed "if mingw" conditionals.
> @rnk about the issue 4.a of https://reviews.llvm.org/D99487#2821343, I tried 
> to use clang-cl.exe with bbs option and lld to build helloworld c program, 
> meet IMAGE_COMDAT_SELECT_XXX related problem.
> helloworld c program is general c demo program as below
> $ cat helloworld.c
> #include 
> 
> int main() {
>printf("hello world!\n");
>return 0;
> }
> $ clang-cl.exe helloworld.c -Xclang -fbasic-block-sections=all -Xclang 
> -funique-basic-block-section-names 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 358481.
TaoPan added a comment.

Change select of BB sections to IMAGE_COMDAT_SELECT_NODUPLICATES


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",discard,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",discard,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",one_only,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.1:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.2:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1733,6 +1733,61 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function , const MachineBasicBlock ,
+const TargetMachine ) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+  StringRef COMDATSymName;
+  if (TM.getUniqueBasicBlockSectionNames())
+COMDATSymName = MBB.getSymbol()->getName();
+  else {
+UniqueID = NextUniqueID++;
+COMDATSymName = 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 356874.
TaoPan added a comment.

git rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",one_only,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.1:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.2:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1733,6 +1733,64 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function , const MachineBasicBlock ,
+const TargetMachine ) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+  StringRef COMDATSymName;
+  if (TM.getUniqueBasicBlockSectionNames())
+COMDATSymName = MBB.getSymbol()->getName();
+  else {
+UniqueID = NextUniqueID++;
+COMDATSymName = MBB.getParent()->getName();
+  }
+
+  // TODO: construct 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 356864.
TaoPan added a comment.

Update dependent D99487 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/test/MC/COFF/seh-bbs.ll

Index: llvm/test/MC/COFF/seh-bbs.ll
===
--- /dev/null
+++ llvm/test/MC/COFF/seh-bbs.ll
@@ -0,0 +1,446 @@
+; RUN: llc -simplifycfg-require-and-preserve-domtree=1 < %s -mtriple=x86_64-windows-msvc -function-sections | llvm-mc -triple=x86_64-windows-msvc -filetype=obj - | llvm-readobj -S --sd --sr -u - | FileCheck --check-prefix=COMMON --check-prefix=BASELINE %s
+; RUN: llc -simplifycfg-require-and-preserve-domtree=1 < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | llvm-mc -triple=x86_64-windows-msvc -filetype=obj - | llvm-readobj -S --sd --sr -u - | FileCheck --check-prefix=COMMON --check-prefix=BBS %s
+
+; COMMON:Sections [
+; COMMON:  Section {
+; COMMON:Name: .xdata
+; COMMON:RawDataSize: 36
+; COMMON:RelocationCount: 4
+; COMMON:Characteristics [
+; COMMON-NEXT: IMAGE_SCN_ALIGN_4BYTES
+; COMMON-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+; COMMON-NEXT: IMAGE_SCN_LNK_COMDAT
+; COMMON-NEXT: IMAGE_SCN_MEM_READ
+; COMMON-NEXT:   ]
+; COMMON:Relocations [
+; COMMON-NEXT: 0xC IMAGE_REL_AMD64_ADDR32NB __C_specific_handler
+; COMMON-NEXT: 0x14 IMAGE_REL_AMD64_ADDR32NB .text
+; COMMON-NEXT: 0x18 IMAGE_REL_AMD64_ADDR32NB .text
+; BASELINE:0x20 IMAGE_REL_AMD64_ADDR32NB .text
+; BBS: 0x20 IMAGE_REL_AMD64_ADDR32NB main.__part.2
+; COMMON:]
+; COMMON:SectionData (
+; COMMON-NEXT: : 190A0345 0A030572 0150 
+; BASELINE-NEXT:   0010: 0100 1200 3400 0100
+; BASELINE-NEXT:   0020: 3500
+; BBS-NEXT:0010: 0100 1200 1100 0100
+; BBS-NEXT:0020: 
+; COMMON-NEXT:   )
+; COMMON-NEXT: }
+; COMMON:  Section {
+; COMMON:Name: .pdata
+; COMMON:RawDataSize: 12
+; COMMON:RelocationCount: 3
+; COMMON:Characteristics [
+; COMMON-NEXT: IMAGE_SCN_ALIGN_4BYTES
+; COMMON-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+; COMMON-NEXT: IMAGE_SCN_LNK_COMDAT
+; COMMON-NEXT: IMAGE_SCN_MEM_READ
+; COMMON-NEXT:   ]
+; COMMON:Relocations [
+; COMMON-NEXT: 0x0 IMAGE_REL_AMD64_ADDR32NB main
+; COMMON-NEXT: 0x4 IMAGE_REL_AMD64_ADDR32NB main
+; COMMON-NEXT: 0x8 IMAGE_REL_AMD64_ADDR32NB .xdata
+; COMMON-NEXT:   ]
+; COMMON:SectionData (
+; BASELINE-NEXT:   :  4F00 
+; BBS-NEXT::  2B00 
+; COMMON-NEXT:   )
+; COMMON-NEXT: }
+; 15 BB Sections and 3 more .xdata and .pdata pairs for function "??0exception@std@@QEAA@QEBD@Z",
+; "??0exception@std@@QEAA@AEBV01@@Z" and "??1exception@std@@UEAA@XZ" in BBS, 21 more sections
+; in Section table of BBS
+; BASELINE:  Number: 45
+; BASELINE-NOT:  Number: 46
+; BBS:   Number: 66
+; BBS-NOT:   Number: 67
+; COMMON:]
+; COMMON:UnwindInformation [
+; COMMON:  RuntimeFunction {
+; COMMON-NEXT:   StartAddress: main (0x0)
+; BASELINE-NEXT: EndAddress: main +0x4F (0x4)
+; BBS-NEXT:  EndAddress: main +0x2B (0x4)
+; COMMON-NEXT:   UnwindInfoAddress: .xdata (0x8)
+; COMMON-NEXT:   UnwindInfo {
+; COMMON-NEXT: Version: 1
+; COMMON-NEXT: Flags [ (0x3)
+; COMMON-NEXT:   ExceptionHandler (0x1)
+; COMMON-NEXT:   TerminateHandler (0x2)
+; COMMON-NEXT: ]
+; COMMON-NEXT: PrologSize: 10
+; COMMON-NEXT: FrameRegister: RBP (0x5)
+; COMMON-NEXT: FrameOffset: 0x4
+; COMMON-NEXT: UnwindCodeCount: 3
+; COMMON-NEXT: UnwindCodes [
+; COMMON-NEXT:   0x0A: SET_FPREG reg=RBP, offset=0x40
+; COMMON-NEXT:   0x05: ALLOC_SMALL size=64
+; COMMON-NEXT:   0x01: PUSH_NONVOL reg=RBP
+; COMMON-NEXT: ]
+; COMMON-NEXT: Handler: __C_specific_handler (0xC)
+; COMMON-NEXT:   }
+; COMMON-NEXT: }
+; COMMON:]
+
+; Generated with this C++ source:
+; #include 
+; #include 
+; #include 
+;
+; int main()
+; {
+; __try
+; {
+; throw std::exception("");
+; }
+; __except(EXCEPTION_EXECUTE_HANDLER)
+; {
+; printf("Executing SEH __except block\r\n");
+; }
+; return 0;
+; }
+
+; ModuleID = 'winseh.cpp'
+source_filename = "winseh.cpp"
+target datalayout = 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

I reported bugs of using clang-cl and lld to test 
https://github.com/microsoft/compiler-tests/tree/master/seh

2. clang-cl.exe + lld linker

  a. x4ptcu.c: build error  https://bugs.llvm.org/show_bug.cgi?id=50859

  b. seh0015.c, seh0017.c: build crash  
https://bugs.llvm.org/show_bug.cgi?id=50854

  c. seh0034.c, seh0036.c, seh0041~0043.c, seh0048~0050.c, another build crash  
https://bugs.llvm.org/show_bug.cgi?id=50855

  d. seh0020.c, seh0025, seh0026: build error  
https://bugs.llvm.org/show_bug.cgi?id=50856

  e. sehframes.cpp: build pass, run dead loop  
https://bugs.llvm.org/show_bug.cgi?id=50858


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

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

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan added a comment.

In D99487#2832079 , @modimo wrote:

> In D99487#2821343 , @TaoPan wrote:
>
>> I checked the microsoft SEH tests with
>>
>> 1. cl.exe
>>
>>   a. x4ptcu.c: build error
>> 2. clang-cl.exe + lld linker
>>
>>   a. x4ptcu.c: build error
>>
>>   b. seh0015.c, seh0017.c: build crash
>>
>>   c. seh0034.c, seh0036.c, seh0041~0043.c, seh0048~0050.c, another build 
>> crash
>>
>>   d. seh0020.c, seh0025, seh0026: build error
>>
>>   e. sehframes.cpp: build pass, run dead loop
>
> Please file bugs for these and if you can bucket the failures that would be 
> even better. Thanks!

I'll file these bugs and try to bucket the failures in parallel.




Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:1712
+  COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_LNK_COMDAT,
+  SectionKind::getText(), COMDATSymName,
+  COFF::IMAGE_COMDAT_SELECT_NODUPLICATES, UniqueID);

TaoPan wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > mstorsjo wrote:
> > > > TaoPan wrote:
> > > > > rnk wrote:
> > > > > > tmsriram wrote:
> > > > > > > MaskRay wrote:
> > > > > > > > TaoPan wrote:
> > > > > > > > > tmsriram wrote:
> > > > > > > > > > COMDATSymName can be folded to be equal to 
> > > > > > > > > > MBB.getSymbol()->getName() here?  Plus, you are not 
> > > > > > > > > > preserving the .text.hot prefix that the original function 
> > > > > > > > > > might get here.  Is this future work?  If the original 
> > > > > > > > > > function is named .text.hot.foo, the basic block will still 
> > > > > > > > > > be named .text.foo.__part.1 which is not right.
> > > > > > > > > > 
> > > > > > > > > > Plus, what about exception handling sections like ".eh.*"?
> > > > > > > > > Thanks! I'll redesign section name and comdat symbol name.
> > > > > > > > > The text section with prefix "hot" and "unlikely" won't be 
> > > > > > > > > constructed here, I added COFF text section prefix "hot" and 
> > > > > > > > > "unlikely" in D92073. In ELF override function, also not 
> > > > > > > > > handling text section with prefix "hot" and "unlikely".
> > > > > > > > > The text section with prefix "split" will be constructed 
> > > > > > > > > here, I plan to add related code in MFS COFF patch.
> > > > > > > > > Also, exception handling section is future work that support 
> > > > > > > > > basic block sections Windows COFF exception handling.
> > > > > > > > This is complex. PE-COFF has multiple COMDAT seletion kinds. I 
> > > > > > > > want to see a holistic plan how every component is going to be 
> > > > > > > > implemented.
> > > > > > > The basic block should just mimic the COMDAT type of its 
> > > > > > > containing function, is there a reason to do anything more with 
> > > > > > > it here?
> > > > > > After thinking about it a bit, I think the entry block should use 
> > > > > > the regular selection kind, and all of the auxilliary MBB sections 
> > > > > > should use IMAGE_COMDAT_SELECT_ASSOCIATIVE. They should be 
> > > > > > associated with the main function symbol, unless the function 
> > > > > > itself is associated with something else, in which case the BBs use 
> > > > > > that symbol.
> > > > > > 
> > > > > > This will ensure that if the main function section prevails, they 
> > > > > > are included, and if it does not prevail, they are discarded. Does 
> > > > > > that make sense?
> > > > > Thanks! I think set entry block sections as regular 
> > > > > IMAGE_COMDAT_SELECT_NODUPLICATES and set the auxilliary MBB sections 
> > > > > as IMAGE_COMDAT_SELECT_ASSOCIATIVE is fine. I changed.
> > > > @rnk - I'm not quite familiar with the concrete implications of this 
> > > > work here, but would this need to be mapped to the GNU style comdats, 
> > > > for compatibility with ld.bfd for mingw targets? (LLD itself works fine 
> > > > with proper associative comdats though.)
> > > I think basic block sections are kind of in the same category as LTO: 
> > > it's a very sophisticated optimization feature, and it's probably OK if 
> > > it's LLD-only. I think it also requires special linker support that might 
> > > make it LLD-only, but I've forgotten the details.
> > > 
> > > It also has potentially very high object file size overhead, and it will 
> > > be important to do everything possible to minimize that object file size 
> > > overhead. Long gnu-style section names for every basic block section are 
> > > likely to make the feature unusably slow, so maybe it's worth removing 
> > > these "if mingw" conditionals. We can leave behind a comment by the use 
> > > of IMAGE_COMDAT_SELECT_ASSOCIATIVE indicating that gnu-style section 
> > > names are not needed because the feature is only designed to work with 
> > > LLD.
> > Thanks for the clarification! Leaving the feature as LLD-only (or in other 
> > terms, requiring a conforming COMDAT implementation) sounds good to me.
> Thanks for discussion between 

[PATCH] D99487: [CodeGen] Port basic block sections from ELF to COFF

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 353844.
TaoPan added a comment.
Herald added subscribers: cfe-commits, dang.
Herald added a project: clang.

Make clang option -fbasic-block-sections and -funique-basic-block-section-names 
available on Windows COFF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99487

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/fbasic-block-sections.c
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/basic-block-sections.ll

Index: llvm/test/CodeGen/X86/basic-block-sections.ll
===
--- llvm/test/CodeGen/X86/basic-block-sections.ll
+++ llvm/test/CodeGen/X86/basic-block-sections.ll
@@ -3,6 +3,10 @@
 ; RUN: llc < %s -mtriple=x86_64-pc-linux -function-sections -basic-block-sections=all -unique-basic-block-section-names -split-machine-functions | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
 ; RUN: llc < %s -mtriple=i386-unknown-linux-gnu  -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=LINUX-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-msvc -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-MSVC-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -function-sections -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-FUNCTION-SECTIONS
+; RUN: llc < %s -mtriple=x86_64-windows-gnu -basic-block-sections=all -unique-basic-block-section-names | FileCheck %s -check-prefix=WINDOWS-GNU-SECTIONS
 
 define void @_Z3bazb(i1 zeroext) nounwind {
   %2 = alloca i8, align 1
@@ -39,3 +43,27 @@
 ; LINUX-SECTIONS: [[SECTION_LABEL_2]]:
 ; LINUX-SECTIONS: .LBB_END0_2:
 ; LINUX-SECTIONS-NEXT: .size   [[SECTION_LABEL_2]], .LBB_END0_2-[[SECTION_LABEL_2]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",one_only,_Z3bazb
+; WINDOWS-MSVC-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-MSVC-SECTIONS: .section.text,"xr"
+; WINDOWS-MSVC-SECTIONS: _Z3bazb:
+; WINDOWS-MSVC-SECTIONS: je  [[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: jmp [[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-MSVC-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text$_Z3bazb,"xr",one_only,_Z3bazb
+; WINDOWS-GNU-FUNCTION-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_1:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_1]]:
+; WINDOWS-GNU-FUNCTION-SECTIONS: .section.text,"xr",associative,[[SECTION_LABEL_2:_Z3bazb.__part.[0-9]+]]
+; WINDOWS-GNU-FUNCTION-SECTIONS: [[SECTION_LABEL_2]]:
+; WINDOWS-GNU-SECTIONS: .section.text$_Z3bazb,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.1:
+; WINDOWS-GNU-SECTIONS: .section.text,"xr"
+; WINDOWS-GNU-SECTIONS: _Z3bazb.__part.2:
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1716,6 +1716,64 @@
   COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID);
 }
 
+/// Returns a unique section for the given machine basic block.
+MCSection *TargetLoweringObjectFileCOFF::getSectionForMachineBasicBlock(
+const Function , const MachineBasicBlock ,
+const TargetMachine ) const {
+  assert(MBB.isBeginSection() && "Basic block does not start a section!");
+  SectionKind Kind = SectionKind::getText();
+  unsigned Characteristics = getCOFFSectionFlags(Kind, TM);
+  // If we have -ffunction-sections then we should emit the global value to a
+  // uniqued section specifically for it.
+  if (TM.getFunctionSections() || F.hasComdat())
+Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT;
+  unsigned UniqueID = MCContext::GenericSectionID;
+  StringRef COMDATSymName;
+  if 

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan added inline comments.



Comment at: clang/test/CodeGen/pre-ra-sched.c:1-2
+// RUN: %clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: %clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+

hubert.reinterpretcast wrote:
> hubert.reinterpretcast wrote:
> > The test as was committed is bogus. It checks the output binary(!) stream 
> > for an error message when stderr was not even redirected. Please fix.
> Fix committed: 
> https://github.com/llvm/llvm-project/commit/603818b97c795114f66a6fc13e8a5f0e54b49a13
Thanks for your review comment and fixing!
I tried to check stderr as you guided and meet empty stdin problem, thanks for 
your quick fixing and I got FileCheck option --allow-empty can resolve this 
kind of problem. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:275-276
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&

pengfei wrote:
> Can this be omitted given `createFastDAGScheduler` should make it linked.
For making scheduler fast and linearize visible by clang -pre-RA-sched, yes, 
createDAGLinearizer can be omitted.
Adding both is also for keeping the ability that all schedulers can be selected 
by TargetLowering object in the case of clang -pre-RA-sched=default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:273-276
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);

craig.topper wrote:
> craig.topper wrote:
> > TaoPan wrote:
> > > pengfei wrote:
> > > > I saw they are always registered in ScheduleDAGFast.cpp:
> > > > https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp#L36
> > > > Why do we register them again here?
> > > Please also help to have a review of the Summary. It's a little bit of a 
> > > trick. ScheduleDAGFast.cpp is compiled to object file, but the object 
> > > file isn't linked into clang executable file as no symbol is referred by 
> > > outside without this patch.
> > That wasn't very clear from your summary. The "the object file isn't linked 
> > into clang executable file as no symbol is referred by outside without this 
> > patch" in this comment was much more help. Can you put something like that 
> > in summary?
> > 
> > Why are they being stripped from clang but not llc?
> Oh I see, llc uses include/llvm/CodeGen/LinkAllCodegenComponents.h to force 
> these to link in.
Thanks! I modified summary. could you please have a review?
Yes, you are right, llc include LinkAllCodegenComponents.h which refers 
function createFastDAGScheduler of ScheduleDAGFast.cpp.



Comment at: llvm/test/CodeGen/Generic/pre-ra-sched.c:1
+// RUN: clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s

craig.topper wrote:
> You can't run clang from an llvm test directory. There's no guarantee clang 
> will have been compiled.
Thanks for notifying no guarantee clang in llvm test directory! I moved the 
test back to clang/test/CodeGen/.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 343800.
TaoPan added a comment.

Move test back to clang/test/CodeGen/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

Files:
  clang/test/CodeGen/pre-ra-sched.c
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp


Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize// Linearize DAG, no scheduling
+};
 
 } // end namespace Sched
 
Index: clang/test/CodeGen/pre-ra-sched.c
===
--- /dev/null
+++ clang/test/CodeGen/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: %clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: %clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: 
Cannot find option named


Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize// Linearize DAG, no scheduling
+};
 
 } // end namespace Sched
 
Index: clang/test/CodeGen/pre-ra-sched.c
===
--- /dev/null
+++ clang/test/CodeGen/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: %clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: %clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for 

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 343598.
TaoPan added a comment.

git rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

Files:
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/test/CodeGen/Generic/pre-ra-sched.c


Index: llvm/test/CodeGen/Generic/pre-ra-sched.c
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: 
Cannot find option named
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize// Linearize DAG, no scheduling
+};
 
 } // end namespace Sched
 


Index: llvm/test/CodeGen/Generic/pre-ra-sched.c
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: Cannot find option named
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize// Linearize 

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan marked an inline comment as done.
TaoPan added a comment.

Thanks Pengfei for your review comments!




Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:273-276
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);

pengfei wrote:
> I saw they are always registered in ScheduleDAGFast.cpp:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGFast.cpp#L36
> Why do we register them again here?
Please also help to have a review of the Summary. It's a little bit of a trick. 
ScheduleDAGFast.cpp is compiled to object file, but the object file isn't 
linked into clang executable file as no symbol is referred by outside without 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan updated this revision to Diff 343589.
TaoPan added a comment.

Move test from clang/test to llvm/test, remove comma of the last enum item.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

Files:
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
  llvm/test/CodeGen/Generic/pre-ra-sched.c


Index: llvm/test/CodeGen/Generic/pre-ra-sched.c
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: 
Cannot find option named
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize// Linearize DAG, no scheduling
+};
 
 } // end namespace Sched
 


Index: llvm/test/CodeGen/Generic/pre-ra-sched.c
===
--- /dev/null
+++ llvm/test/CodeGen/Generic/pre-ra-sched.c
@@ -0,0 +1,4 @@
+// RUN: clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: Cannot find option named
Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,   

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan added reviewers: RKSimon, craig.topper.
TaoPan added a comment.

Could you please have a review?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101601

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

[PATCH] D101601: [SelectionDAG] Make fast and linearize visible by clang -pre-RA-sched

[TaoPan via Phabricator via cfe-commits]

TaoPan created this revision.
Herald added subscribers: ecnelises, hiraditya.
TaoPan requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Scheduler can be registered through static RegisterScheduler if the host
cpp file is linked, the schedulers of SchedulerDAGRRList.cpp and
SchedulerDAGVLIW.cpp are visible as functions createXxx of these two cpp
files are called by SelectionDAGISel.cpp, add calling to createXxx of
ScheduleDAGFast.cpp to make scheduler fast and linearize visible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101601

Files:
  clang/test/CodeGen/pre-ra-sched.c
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp


Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize,   // Linearize DAG, no scheduling
+};
 
 } // end namespace Sched
 
Index: clang/test/CodeGen/pre-ra-sched.c
===
--- /dev/null
+++ clang/test/CodeGen/pre-ra-sched.c
@@ -0,0 +1,8 @@
+// RUN: %clang %s -mllvm -pre-RA-sched=fast -c -o - | FileCheck %s
+// RUN: %clang %s -mllvm -pre-RA-sched=linearize -c -o - | FileCheck %s
+
+// CHECK-NOT: clang (LLVM option parsing): for the --pre-RA-sched option: 
Cannot find option named
+
+int main() {
+  return 0;
+}


Index: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
===
--- llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -270,6 +270,10 @@
   return createHybridListDAGScheduler(IS, OptLevel);
 if (TLI->getSchedulingPreference() == Sched::VLIW)
   return createVLIWDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Fast)
+  return createFastDAGScheduler(IS, OptLevel);
+if (TLI->getSchedulingPreference() == Sched::Linearize)
+  return createDAGLinearizer(IS, OptLevel);
 assert(TLI->getSchedulingPreference() == Sched::ILP &&
"Unknown sched type!");
 return createILPListDAGScheduler(IS, OptLevel);
Index: llvm/include/llvm/CodeGen/TargetLowering.h
===
--- llvm/include/llvm/CodeGen/TargetLowering.h
+++ llvm/include/llvm/CodeGen/TargetLowering.h
@@ -94,14 +94,16 @@
 
 namespace Sched {
 
-  enum Preference {
-None, // No preference
-Source,   // Follow source order.
-RegPressure,  // Scheduling for lowest register pressure.
-Hybrid,   // Scheduling for both latency and register pressure.
-ILP,  // Scheduling for ILP in low register pressure mode.
-VLIW  // Scheduling for VLIW targets.
-  };
+enum Preference {
+  None,// No preference
+  Source,  // Follow source order.
+  RegPressure, // Scheduling for lowest register pressure.
+  Hybrid,  // Scheduling for both latency and register pressure.
+  ILP, // Scheduling for ILP in low register pressure mode.
+  VLIW,// Scheduling for VLIW targets.
+  Fast,// Fast suboptimal list scheduling
+  Linearize,   // Linearize DAG, no