[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-03-22 Thread James Y Knight via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rC356789: IR: Support parsing numeric block ids, and emit them 
in textual output. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58548?vs=187994&id=191913#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58548

Files:
  test/CodeGenCXX/discard-name-values.cpp


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-03-18 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-03-18 Thread James Y Knight via Phabricator via cfe-commits 
jyknight added a comment.

Sorry, forgot to re-ping this in a timely manner. :)

The discussion on mailing list concluded positively, so waiting for an LG here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits 
jyknight updated this revision to Diff 187994.
jyknight marked 4 inline comments as done.
jyknight added a comment.

Minor tweaks per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a comment.

I have a few nitpicks, but otherwise this seems right.  I'll wait for the 
llvm-dev discussion before LGTM'ing though.




Comment at: llvm/lib/AsmParser/LLParser.cpp:2930-2931
+if (NameID != -1 && unsigned(NameID) != NumberedVals.size()) {
+  P.Error(Loc, "label expected to be numbered '%" +
+   Twine(NumberedVals.size()) + "'");
+  return nullptr;

Since the label doesn't include a `%`, I think you should drop that part of the 
error message.



Comment at: llvm/lib/AsmParser/LLParser.cpp:2936-2937
+if (!BB) {
+  P.Error(Loc,
+  "unable to create block numbered " + Twine(NumberedVals.size()));
+  return nullptr;

I think single quotes around the number would be better here.



Comment at: llvm/lib/AsmParser/LLParser.cpp:5580-5581
   switch (Token) {
-  default:return Error(Loc, "expected instruction opcode");
+  default:
+return Error(Loc, "expected instruction opcode");
   // Terminator Instructions.

Looks unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits 
jyknight added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.  
> A few things:
>
> - Changes to the IR should always be discussed on llvm-dev.  Did this already 
> happen?


I sent a message ("Improving textual IR format for nameless blocks") 
concurrently with sending this review, and will wait a bit to make sure there's 
no objections.

> - Please update LangRef.

Done.

> - Did you write a script for updating the test cases?  If so, you might 
> consider attaching it to the RFC and linking to it from the commit message as 
> a courtesy to downstream maintainers.

Nope, I didn't; there were few enough instances that it didn't seem worth 
scripting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits 
jyknight updated this revision to Diff 187963.
jyknight added a comment.

Add some wording to LangRef and clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ llvm/test/Instrumentation/Sanitize

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a comment.

In D58548#1407355 , @dexonsmith wrote:

> In D58548#1407331 , @greened wrote:
>
> > +1.  Is there any reason not to use "%4" in the definition?
> >
> >   define i32 @f(i32, i32) {
> > %3 = add i32 %0, %1
> > br label %4
> >  
> >   %4:
> > ret i32 %3
> >   }
> >
> >
> > Maybe it creates an ambiguity in the grammar or something.
>
>
> A `%` isn't used for labels that do have names, so David's approach seems 
> consistent.


s/David/James/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a comment.

In D58548#1407331 , @greened wrote:

> +1.  Is there any reason not to use "%4" in the definition?
>
>   define i32 @f(i32, i32) {
> %3 = add i32 %0, %1
> br label %4
>  
>   %4:
> ret i32 %3
>   }
>
>
> Maybe it creates an ambiguity in the grammar or something.


A `%` isn't used for labels that do have names, so David's approach seems 
consistent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread David Greene via Phabricator via cfe-commits 
greened added a comment.

+1.  Is there any reason not to use "%4" in the definition?

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  %4:
ret i32 %3
  }

Maybe it creates an ambiguity in the grammar or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.


Textual IR *change*; typing on a phone while walking :/.  By which I mean that 
IMO it’s fine to break/change this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits 
dexonsmith added a comment.

I like this idea, and I don’t think the textual IR central is too important.  A 
few things:

- Changes to the IR should always be discussed on llvm-dev.  Did this already 
happen?
- Please update LangRef.
- Did you write a script for updating the test cases?  If so, you might 
consider attaching it to the RFC and linking to it from the commit message as a 
courtesy to downstream maintainers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits 
jyknight created this revision.
jyknight added a reviewer: philip.pfaffe.
Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, 
hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Just as as llvm IR supports explicitly specifying numeric value ids
for instructions, and emits them by default in textual output, now do
the same for blocks.

This is a slightly incompatible change in the textual IR format.

Previously, llvm would parse numeric labels as string names. E.g.

  define void @f() {
br label %"55"
  55:
ret void
  }

defined a label *named* "55", even without needing to be quoted, while
the reference required quoting. Now, if you intend a block label which
looks like a value number to be a name, you must quote it in the
definition too (e.g. `"55":`).

Previously, llvm would print nameless blocks only as a comment, and
would omit it if there was no predecessor. This could cause confusion
for readers of the IR, just as unnamed instructions did prior to the
addition of "%5 = " syntax, back in 2008 (PR2480).

Now, it will always print a label for an unnamed block, with the
exception of the entry block. (IMO it may be better to print it for
the entry-block as well. However, that requires updating many more
tests.)

Thus, the following is supported, and is the canonical printing:

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  4:
ret i32 %3
  }

New test cases covering this behavior are added, and other tests
updated as required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1