Merge branch 'main' into hgh/libcxx/P2944R3-optional-constrained-equality

Author: H-G-Hristov

clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp

@@ -104,6 +104,7 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
       return builder.createBoolToInt(value, dstTy);
     if (mlir::isa<cir::BoolType>(dstTy))
       return value;
+    llvm_unreachable("Can only promote integer or boolean types");
   }
 
   //===--------------------------------------------------------------------===//
@@ -1857,9 +1858,6 @@ mlir::Value ScalarExprEmitter::VisitUnaryLNot(const UnaryOperator *e) {
 
   // ZExt result to the expr type.
   return maybePromoteBoolResult(boolVal, cgf.convertType(e->getType()));
-
-  cgf.cgm.errorNYI("destination type for logical-not unary operator is NYI");
-  return {};
 }
 
 /// Return the size or alignment of the type of argument of the sizeof

clang/lib/CodeGen/CGStmt.cpp

@@ -1483,7 +1483,14 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
   if (!Weights && CGM.getCodeGenOpts().OptimizationLevel)
     BoolCondVal = emitCondLikelihoodViaExpectIntrinsic(
         BoolCondVal, Stmt::getLikelihood(S.getBody()));
-  Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock, Weights);
+  auto *I = Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock, Weights);
+  // Key Instructions: Emit the condition and branch as separate atoms to
+  // match existing loop stepping behaviour. FIXME: We could have the branch as
+  // the backup location for the condition, which would probably be a better
+  // experience.
+  if (auto *CondI = dyn_cast<llvm::Instruction>(BoolCondVal))
+    addInstToNewSourceAtom(CondI, nullptr);
+  addInstToNewSourceAtom(I, nullptr);
 
   if (ExitBlock != LoopExit.getBlock()) {
     EmitBlock(ExitBlock);
@@ -1508,6 +1515,9 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
     EmitStmt(S.getLoopVarStmt());
     EmitStmt(S.getBody());
   }
+  // The last block in the loop's body (which unconditionally branches to the
+  // `inc` block if there is one).
+  auto *FinalBodyBB = Builder.GetInsertBlock();
 
   EmitStopPoint(&S);
   // If there is an increment, emit it next.
@@ -1532,6 +1542,12 @@ CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
 
   if (CGM.shouldEmitConvergenceTokens())
     ConvergenceTokenStack.pop_back();
+
+  if (FinalBodyBB) {
+    // We want the for closing brace to be step-able on to match existing
+    // behaviour.
+    addInstToNewSourceAtom(FinalBodyBB->getTerminator(), nullptr);
+  }
 }
 
 void CodeGenFunction::EmitReturnOfRValue(RValue RV, QualType Ty) {

clang/test/DebugInfo/KeyInstructions/for-range.cpp

@@ -0,0 +1,99 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu  -gkey-instructions %s -debug-info-kind=line-tables-only -emit-llvm -o - \
+// RUN: | FileCheck %s
+
+// Perennial question: should the inc be its own source atom or not
+// (currently it is).
+
+// FIXME: See do.c and while.c regarding cmp and cond br groups.
+
+// The stores in the setup (stores to __RANGE1, __BEGIN1, __END1) are all
+// marked as Key. Unclear whether that's desirable. Keep for now as that's
+// least risky (at worst it introduces an unnecessary step while debugging,
+// as opposed to potentially losing one we want).
+
+// Check the conditional branch (and the condition) in FOR_COND and
+// unconditional branch in FOR_BODY are Key Instructions.
+
+struct Range {
+    int *begin();
+    int *end();
+} r;
+
+// CHECK-LABEL: define dso_local void @_Z1av(
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] !dbg [[DBG10:![0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[__RANGE1:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[__BEGIN1:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[__END1:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[I:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store ptr @r, ptr [[__RANGE1]], align 8, !dbg [[DBG14:![0-9]+]]
+// CHECK-NEXT:    [[CALL:%.*]] = call noundef ptr @_ZN5Range5beginEv(ptr noundef nonnull align 1 dereferenceable(1) @r), !dbg [[DBG15:![0-9]+]]
+// CHECK-NEXT:    store ptr [[CALL]], ptr [[__BEGIN1]], align 8, !dbg [[DBG16:![0-9]+]]
+// CHECK-NEXT:    [[CALL1:%.*]] = call noundef ptr @_ZN5Range3endEv(ptr noundef nonnull align 1 dereferenceable(1) @r), !dbg [[DBG17:![0-9]+]]
+// CHECK-NEXT:    store ptr [[CALL1]], ptr [[__END1]], align 8, !dbg [[DBG18:![0-9]+]]
+// CHECK-NEXT:    br label %[[FOR_COND:.*]], !dbg [[DBG19:![0-9]+]]
+// CHECK:       [[FOR_COND]]:
+// CHECK-NEXT:    [[TMP0:%.*]] = load ptr, ptr [[__BEGIN1]], align 8, !dbg [[DBG19]]
+// CHECK-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[__END1]], align 8, !dbg [[DBG19]]
+// CHECK-NEXT:    [[CMP:%.*]] = icmp ne ptr [[TMP0]], [[TMP1]], !dbg [[DBG20:![0-9]+]]
+// CHECK-NEXT:    br i1 [[CMP]], label %[[FOR_BODY:.*]], label %[[FOR_END:.*]], !dbg [[DBG21:![0-9]+]]
+// CHECK:       [[FOR_BODY]]:
+// CHECK-NEXT:    [[TMP2:%.*]] = load ptr, ptr [[__BEGIN1]], align 8, !dbg [[DBG19]]
+// CHECK-NEXT:    [[TMP3:%.*]] = load i32, ptr [[TMP2]], align 4, !dbg [[DBG22:![0-9]+]]
+// CHECK-NEXT:    store i32 [[TMP3]], ptr [[I]], align 4, !dbg [[DBG23:![0-9]+]]
+// CHECK-NEXT:    br label %[[FOR_INC:.*]], !dbg [[DBG24:![0-9]+]]
+// CHECK:       [[FOR_INC]]:
+// CHECK-NEXT:    [[TMP4:%.*]] = load ptr, ptr [[__BEGIN1]], align 8, !dbg [[DBG19]]
+// CHECK-NEXT:    [[INCDEC_PTR:%.*]] = getelementptr inbounds nuw i32, ptr [[TMP4]], i32 1, !dbg [[DBG25:![0-9]+]]
+// CHECK-NEXT:    store ptr [[INCDEC_PTR]], ptr [[__BEGIN1]], align 8, !dbg [[DBG26:![0-9]+]]
+// CHECK-NEXT:    br label %[[FOR_COND]], !dbg [[DBG19]], !llvm.loop
+// CHECK:       [[FOR_END]]:
+// CHECK-NEXT:    ret void, !dbg [[DBG30:![0-9]+]]
+//
+void a() {
+  for (int i: r)
+    ;
+}
+
+// - Check the branch out of the body gets an atom group (and gets it correct
+// if there's ctrl-flow in the body).
+void b() {
+  for (int i: r) {
+    if (i)
+      ;
+// CHECK: entry:
+// CHECK: if.end:
+// CHECK-NEXT: br label %for.inc, !dbg [[b_br:!.*]]
+  }
+}
+
+// - Check an immediate loop exit (`break` in this case) gets an atom group and
+// - doesn't cause a crash.
+void c() {
+  for (int i: r)
+// CHECK: entry:
+// CHECK: for.body:
+// CHECK: br label %for.end, !dbg [[c_br:!.*]]
+    break;
+}
+
+//.
+// CHECK: [[DBG14]] = !DILocation({{.*}}, atomGroup: 1, atomRank: 1)
+// CHECK: [[DBG15]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 2)
+// CHECK: [[DBG16]] = !DILocation({{.*}}, atomGroup: 2, atomRank: 1)
+// CHECK: [[DBG17]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 2)
+// CHECK: [[DBG18]] = !DILocation({{.*}}, atomGroup: 3, atomRank: 1)
+// CHECK: [[DBG19]] = !DILocation({{.*}})
+// CHECK: [[DBG20]] = !DILocation({{.*}}, atomGroup: 4, atomRank: 1)
+// CHECK: [[DBG21]] = !DILocation({{.*}}, atomGroup: 5, atomRank: 1)
+// CHECK: [[DBG22]] = !DILocation({{.*}}, atomGroup: 6, atomRank: 2)
+// CHECK: [[DBG23]] = !DILocation({{.*}}, atomGroup: 6, atomRank: 1)
+// CHECK: [[DBG24]] = !DILocation({{.*}} atomGroup: 8, atomRank: 1)
+// CHECK: [[DBG25]] = !DILocation({{.*}}, atomGroup: 7, atomRank: 2)
+// CHECK: [[DBG26]] = !DILocation({{.*}}, atomGroup: 7, atomRank: 1)
+// CHECK: [[DBG30]] = !DILocation({{.*}})
+//
+// CHECK: [[b_br]] = !DILocation({{.*}}, atomGroup: [[#]], atomRank: [[#]])
+//
+// CHECK: [[c_br]] = !DILocation({{.*}}, atomGroup: [[#]], atomRank: [[#]])
+//.

lldb/source/Commands/CommandObjectProcess.cpp

@@ -1303,7 +1303,7 @@ class CommandObjectProcessSaveCore : public CommandObjectParsed {
         llvm_unreachable("Unimplemented option");
       }
 
-      return {};
+      return error;
     }
 
     void OptionParsingStarting(ExecutionContext *execution_context) override {

lldb/source/Symbol/SaveCoreOptions.cpp

@@ -24,7 +24,6 @@ Status SaveCoreOptions::SetPluginName(const char *name) {
   if (!PluginManager::IsRegisteredObjectFilePluginName(name)) {
     return Status::FromErrorStringWithFormat(
         "plugin name '%s' is not a valid ObjectFile plugin name", name);
-    return error;
   }
 
   m_plugin_name = name;

lldb/test/Shell/Commands/command-process-save-core-not-a-plugin.test

@@ -0,0 +1,19 @@
+# This checks that lldb returns an error if process save-core is called
+# with a plugin that does not exist.
+
+# RUN: %clang_host -g %S/Inputs/main.c -o %t
+# RUN: %lldb %t -s %s -o exit 2>&1 | FileCheck %s
+
+b main
+# CHECK-LABEL: b main
+# CHECK: Breakpoint 1: where = {{.*}}`main
+
+run
+# CHECK-LABEL: run
+# CHECK: Process {{.*}} stopped
+# CHECK: stop reason = breakpoint 1
+# CHECK:   frame #0: {{.*}}`main at main.c
+
+process save-core --plugin-name=notaplugin dump
+# CHECK-LABEL: process save-core --plugin-name=notaplugin dump
+# CHECK: error: plugin name 'notaplugin' is not a valid ObjectFile plugin name

llvm/docs/CommandGuide/lit.rst

@@ -63,12 +63,12 @@ GENERAL OPTIONS
 
  Show :program:`lit`'s version number and exit.
 
-.. option:: -j N, --workers=N
+.. option:: -j N, --workers N
 
  Run ``N`` tests in parallel.  By default, this is automatically chosen to
  match the number of detected available CPUs.
 
-.. option:: --config-prefix=NAME
+.. option:: --config-prefix NAME
 
  Search for :file:`{NAME}.cfg` and :file:`{NAME}.site.cfg` when searching for
  test suites, instead of :file:`lit.cfg` and :file:`lit.site.cfg`.
@@ -157,7 +157,7 @@ EXECUTION OPTIONS
 
  Disable sharding for GoogleTest format.
 
-.. option:: --path=PATH
+.. option:: --path PATH
 
  Specify an additional ``PATH`` to use when searching for executables in tests.
 
@@ -178,7 +178,7 @@ EXECUTION OPTIONS
  feature that can be used to conditionally disable (or expect failure in)
  certain tests.
 
-.. option:: --vg-arg=ARG
+.. option:: --vg-arg ARG
 
  When :option:`--vg` is used, specify an additional argument to pass to
  :program:`valgrind` itself.
@@ -208,7 +208,7 @@ EXECUTION OPTIONS
  Maximum time to spend running a single test (in seconds). 0 means no time
  limit. [Default: 0]
 
-.. option:: --timeout=N
+.. option:: --timeout N
 
  Spend at most ``N`` seconds (approximately) running each individual test.
  ``0`` means no time limit, and ``0`` is the default. Note that this is not an
@@ -268,17 +268,17 @@ The timing data is stored in the `test_exec_root` in a file named
 `.lit_test_times.txt`. If this file does not exist, then `lit` checks the
 `test_source_root` for the file to optionally accelerate clean builds.
 
-.. option:: --max-tests=N
+.. option:: --max-tests N
 
  Run at most ``N`` tests and then terminate.
 
-.. option:: --max-time=N
+.. option:: --max-time N
 
  Spend at most ``N`` seconds (approximately) running tests and then terminate.
  Note that this is not an alias for :option:`--timeout`; the two are
  different kinds of maximums.
 
-.. option:: --order={lexical,random,smart}
+.. option:: --order {lexical,random,smart}
 
  Define the order in which tests are run. The supported values are:
 
@@ -300,21 +300,21 @@ The timing data is stored in the `test_exec_root` in a file named
 
  Run failed tests first (DEPRECATED: use ``--order=smart``).
 
-.. option:: --filter=REGEXP
+.. option:: --filter REGEXP
 
   Run only those tests whose name matches the regular expression specified in
   ``REGEXP``. The environment variable ``LIT_FILTER`` can be also used in place
   of this option, which is especially useful in environments where the call
   to ``lit`` is issued indirectly.
 
-.. option:: --filter-out=REGEXP
+.. option:: --filter-out REGEXP
 
   Filter out those tests whose name matches the regular expression specified in
   ``REGEXP``. The environment variable ``LIT_FILTER_OUT`` can be also used in
   place of this option, which is especially useful in environments where the
   call to ``lit`` is issued indirectly.
 
-.. option:: --xfail=LIST
+.. option:: --xfail LIST
 
   Treat those tests whose name is in the semicolon separated list ``LIST`` as
   ``XFAIL``. This can be helpful when one does not want to modify the test
@@ -346,7 +346,7 @@ The timing data is stored in the `test_exec_root` in a file named
 
     LIT_XFAIL="affinity/kmp-hw-subset.c;libomptarget :: x86_64-pc-linux-gnu :: offloading/memory_manager.cpp"
 
-.. option:: --xfail-not=LIST
+.. option:: --xfail-not LIST
 
   Do not treat the specified tests as ``XFAIL``.  The environment variable
   ``LIT_XFAIL_NOT`` can also be used in place of this option.  The syntax is the
@@ -356,7 +356,7 @@ The timing data is stored in the `test_exec_root` in a file named
   primary purpose is to suppress an ``XPASS`` result without modifying a test
   case that uses the ``XFAIL`` directive.
 
-.. option:: --num-shards=M
+.. option:: --num-shards M
 
  Divide the set of selected tests into ``M`` equal-sized subsets or
  "shards", and run only one of them.  Must be used with the
@@ -366,7 +366,7 @@ The timing data is stored in the `test_exec_root` in a file named
  testsuites, for parallel execution on separate machines (say in a large
  testing farm).
 
-.. option:: --run-shard=N
+.. option:: --run-shard N
 
  Select which shard to run, assuming the ``--num-shards=M`` option was
  provided. The two options must be used together, and the value of ``N``