[clang] [Coverage] Suppress covmap and profdata for system headers. (PR #97952)

2024-07-08 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/97952
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-11 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.


https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-11 Thread Jessica Paquette via cfe-commits


@@ -484,10 +484,31 @@ MC/DC Instrumentation
 -
 
 When instrumenting for Modified Condition/Decision Coverage (MC/DC) using the
-clang option ``-fcoverage-mcdc``, users are limited to at most **six** 
leaf-level
-conditions in a boolean expression.  A warning will be generated for boolean
-expressions that contain more than six, and they will not be instrumented for
-MC/DC.
+clang option ``-fcoverage-mcdc``, there are two hard limits.

ornata wrote:

I think it's fine actually.

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-10 Thread Jessica Paquette via cfe-commits


@@ -484,10 +484,31 @@ MC/DC Instrumentation
 -
 
 When instrumenting for Modified Condition/Decision Coverage (MC/DC) using the
-clang option ``-fcoverage-mcdc``, users are limited to at most **six** 
leaf-level
-conditions in a boolean expression.  A warning will be generated for boolean
-expressions that contain more than six, and they will not be instrumented for
-MC/DC.
+clang option ``-fcoverage-mcdc``, there are two hard limits.
+
+The maximum number of terms is limited to 32767, which is practical for
+handwritten expressions. To be more restrictive in order to enforce coding 
rules,
+use ``-Xclang -fmcdc-max-conditions=n``. Expressions with exceeded condition
+counts ``n`` will generate warnings.
+
+The number of test vectors (the maximum number of possible combinations of
+expressions) is limited to 2,147,483,646. In this case, approximately
+256MiB (==2GiB/8) is used to record test vectors.
+
+To reduce memory usage, you can limit the maximum number of test vectors per

ornata wrote:

Suggest consistently using "user" or "you" throughout docs.

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-10 Thread Jessica Paquette via cfe-commits

https://github.com/ornata commented:

Some nits on documentation wording. Code looks fine to me.

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-10 Thread Jessica Paquette via cfe-commits

https://github.com/ornata edited https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-06-10 Thread Jessica Paquette via cfe-commits


@@ -484,10 +484,31 @@ MC/DC Instrumentation
 -
 
 When instrumenting for Modified Condition/Decision Coverage (MC/DC) using the
-clang option ``-fcoverage-mcdc``, users are limited to at most **six** 
leaf-level
-conditions in a boolean expression.  A warning will be generated for boolean
-expressions that contain more than six, and they will not be instrumented for
-MC/DC.
+clang option ``-fcoverage-mcdc``, there are two hard limits.

ornata wrote:

Suggest simplifying the phrasing here.

> By default, Modified Condition/Decision Coverage (MC/DC) instrumentation is 
> limited to 32767 terms. Users may provide their own limit on the number of 
> terms, `n` by passing ``-Xclang -fmcdc-max-conditions=n``.

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [MC/DC][Coverage] Add assertions into emitSourceRegions() (PR #89572)

2024-05-22 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/89572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Coverage][Expansion] handle nested macros in scratch space (PR #89869)

2024-05-22 Thread Jessica Paquette via cfe-commits


@@ -517,7 +552,7 @@ class CoverageMappingBuilder {
 SourceRegionFilter Filter;
 for (const auto &FM : FileIDMapping) {
   SourceLocation ExpandedLoc = FM.second.second;
-  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);

ornata wrote:

I thought it was only used in one place, oops. :) The current way is fine.

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Coverage] Rework !SystemHeadersCoverage (PR #91446)

2024-05-20 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/91446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [MC/DC][Coverage] Add assertions into emitSourceRegions() (PR #89572)

2024-05-19 Thread Jessica Paquette via cfe-commits


@@ -190,6 +190,16 @@ class SourceMappingRegion {
 
   bool isBranch() const { return FalseCount.has_value(); }
 
+  bool isMCDCBranch() const {
+const auto *BranchParams = 
std::get_if(&MCDCParams);
+assert(BranchParams == nullptr || BranchParams->ID >= 0);
+return (BranchParams != nullptr);
+  }
+
+  const auto &getMCDCBranchParams() const {

ornata wrote:

unused function?

https://github.com/llvm/llvm-project/pull/89572
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Coverage] Rework !SystemHeadersCoverage (PR #91446)

2024-05-19 Thread Jessica Paquette via cfe-commits


@@ -2064,7 +2082,20 @@ struct CounterCoverageMappingBuilder
 createDecisionRegion(E, DecisionParams);
   }
 
+  /// Check if E belongs to system headers.
+  bool isExprInSystemHeader(const BinaryOperator *E) const {

ornata wrote:

assert E is not nullptr?

https://github.com/llvm/llvm-project/pull/91446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [Coverage][Expansion] handle nested macros in scratch space (PR #89869)

2024-05-19 Thread Jessica Paquette via cfe-commits


@@ -517,7 +552,7 @@ class CoverageMappingBuilder {
 SourceRegionFilter Filter;
 for (const auto &FM : FileIDMapping) {
   SourceLocation ExpandedLoc = FM.second.second;
-  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc);
+  SourceLocation ParentLoc = getIncludeOrExpansionLoc(ExpandedLoc, false);

ornata wrote:

The boolean parameter for this function is only used in this single callsite. 
Do we need it at all?

https://github.com/llvm/llvm-project/pull/89869
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-04-17 Thread Jessica Paquette via cfe-commits


@@ -983,7 +979,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
   // for most embedded applications. Setting a maximum value prevents the
   // bitmap footprint from growing too large without the user's knowledge. In
   // the future, this value could be adjusted with a command-line option.
-  unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage) ? 6 : 0;
+  unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage) ? 32767 : 0;

ornata wrote:

Also why 32767? Max signed 16-bit value? Is there a reason?

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-04-17 Thread Jessica Paquette via cfe-commits


@@ -2050,23 +2069,74 @@ struct CounterCoverageMappingBuilder
  subtractCounters(ParentCount, TrueCount));
   }
 
-  void createDecision(const BinaryOperator *E) {
+  void createOrCancelDecision(const BinaryOperator *E, unsigned Since) {
 unsigned NumConds = MCDCBuilder.getTotalConditionsAndReset(E);
 if (NumConds == 0)
   return;
 
+// Extract [ID, Conds] to construct the graph.
+llvm::SmallVector CondIDs(NumConds);
+for (const auto &SR : ArrayRef(SourceRegions).slice(Since)) {
+  if (SR.isMCDCBranch()) {

ornata wrote:

nit: can you use `make_filter_range` from `STLExtras`?

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-04-17 Thread Jessica Paquette via cfe-commits


@@ -190,18 +190,30 @@ class SourceMappingRegion {
 
   bool isBranch() const { return FalseCount.has_value(); }
 
+  bool isMCDCBranch() const {
+const auto *BranchParams = 
std::get_if(&MCDCParams);
+assert(BranchParams == nullptr || BranchParams->ID >= 0);
+return (BranchParams != nullptr);
+  }
+
+  const auto &getMCDCBranchParams() const {
+return mcdc::getParams(MCDCParams);
+  }
+
   bool isMCDCDecision() const {
 const auto *DecisionParams =
 std::get_if(&MCDCParams);
-assert(!DecisionParams || DecisionParams->NumConditions > 0);
-return DecisionParams;
+assert(DecisionParams == nullptr || DecisionParams->NumConditions > 0);

ornata wrote:

Similar to the other comment, it would be nice if we could enforce at 
construction-time that MC/DC parameters have at least 1 condition.

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-04-17 Thread Jessica Paquette via cfe-commits


@@ -190,18 +190,30 @@ class SourceMappingRegion {
 
   bool isBranch() const { return FalseCount.has_value(); }
 
+  bool isMCDCBranch() const {
+const auto *BranchParams = 
std::get_if(&MCDCParams);
+assert(BranchParams == nullptr || BranchParams->ID >= 0);

ornata wrote:

I think it would be nice if this assert was not necessary. Is it possible to 
enforce that BranchParams->ID is never when we have MC/DC parameters?

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC][Coverage] Loosen the limit of NumConds from 6 (PR #82448)

2024-02-20 Thread Jessica Paquette via cfe-commits


@@ -983,7 +979,7 @@ void CodeGenPGO::mapRegionCounters(const Decl *D) {
   // for most embedded applications. Setting a maximum value prevents the
   // bitmap footprint from growing too large without the user's knowledge. In
   // the future, this value could be adjusted with a command-line option.
-  unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage) ? 6 : 0;
+  unsigned MCDCMaxConditions = (CGM.getCodeGenOpts().MCDCCoverage) ? 32767 : 0;

ornata wrote:

I think the limit should be controlled by a flag, with a reasonable default 
value.

E.g.

`-mcdc-max-conditions=`

https://github.com/llvm/llvm-project/pull/82448
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC] Refactor: Let MCDCConditionID int16_t with zero-origin (PR #81257)

2024-02-14 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.


https://github.com/llvm/llvm-project/pull/81257
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] clangCodeGen: Introduce `mcdc::State` with `MCDCState.h` (PR #81497)

2024-02-13 Thread Jessica Paquette via cfe-commits


@@ -0,0 +1,33 @@
+//=== MCDCState.h - MC/DC-related types for PGO -*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+//  MC/DC-related types for PGO

ornata wrote:

comment seems misleading; this is only for the per-function MC/DC state, right?

https://github.com/llvm/llvm-project/pull/81497
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC] Refactor: Introduce `MCDCTypes.h` for `coverage::mcdc` (PR #81459)

2024-02-13 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/81459
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC] Refactor: Make `MCDCParams` as `std::variant` (PR #81227)

2024-02-13 Thread Jessica Paquette via cfe-commits

https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/81227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC] Refactor: Make `MCDCParams` as `std::variant` (PR #81227)

2024-02-11 Thread Jessica Paquette via cfe-commits

ornata wrote:

Is the main benefit of this avoiding zero initialization?

https://github.com/llvm/llvm-project/pull/81227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [MC/DC] Refactor: Make `MCDCParams` as `std::variant` (PR #81227)

2024-02-11 Thread Jessica Paquette via cfe-commits


@@ -308,13 +309,21 @@ Error 
RawCoverageMappingReader::readMappingRegionsSubArray(
 return Err;
   if (auto Err = readIntMax(FID, std::numeric_limits::max()))
 return Err;
+  if (ID == 0)
+return make_error(
+coveragemap_error::malformed,
+"MCDCConditionID shouldn't be zero");
+  Params = CounterMappingRegion::MCDCBranchParameters{
+  (unsigned)ID, (unsigned)TID, (unsigned)FID};

ornata wrote:

`static_cast`?

https://github.com/llvm/llvm-project/pull/81227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' (PR #78033)

2024-01-22 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= ,
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= ,
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= ,
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= ,
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= 
Message-ID:
In-Reply-To: 


https://github.com/ornata approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' (PR #78033)

2024-01-22 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= 
Message-ID:
In-Reply-To: 



@@ -174,6 +179,10 @@ class SourceMappingRegion {
 
   void setGap(bool Gap) { GapRegion = Gap; }
 
+  bool isSkipped() const { return SkippedRegion; }

ornata wrote:

Unrelated to this patch: I wonder if `SkippedRegion` is being used elsewhere. 
This could be nice clean up.

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' (PR #78033)

2024-01-22 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= 
Message-ID:
In-Reply-To: 



@@ -1251,6 +1264,69 @@ struct CounterCoverageMappingBuilder
 popRegions(Index);
   }
 
+  /// Find a valid range starting with \p StartingLoc and ending before \p
+  /// BeforeLoc.
+  std::optional findAreaStartingFromTo(SourceLocation StartingLoc,
+SourceLocation BeforeLoc) {
+// If StartingLoc is in function-like macro, use its start location.
+if (StartingLoc.isMacroID()) {
+  FileID FID = SM.getFileID(StartingLoc);
+  const SrcMgr::ExpansionInfo *EI = &SM.getSLocEntry(FID).getExpansion();
+  if (EI->isFunctionMacroExpansion())
+StartingLoc = EI->getExpansionLocStart();
+}
+
+size_t StartDepth = locationDepth(StartingLoc);
+size_t EndDepth = locationDepth(BeforeLoc);
+while (!SM.isWrittenInSameFile(StartingLoc, BeforeLoc)) {
+  bool UnnestStart = StartDepth >= EndDepth;
+  bool UnnestEnd = EndDepth >= StartDepth;
+  if (UnnestEnd) {

ornata wrote:

Could you add a comment explaining `UnnestStart` and `UnnestEnd`?

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' (PR #78033)

2024-01-22 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= 
Message-ID:
In-Reply-To: 



@@ -1700,43 +1776,116 @@ struct CounterCoverageMappingBuilder
 Visit(S->getSubStmt());
   }
 
+  void CoverIfConsteval(const IfStmt *S) {
+assert(S->isConsteval());
+
+const auto *Then = S->getThen();
+const auto *Else = S->getElse();
+
+// I'm using 'propagateCounts' later as new region is better and allows me
+// to properly calculate line coverage in llvm-cov utility
+const Counter ParentCount = getRegion().getCounter();
+
+extendRegion(S);
+
+if (S->isNegatedConsteval()) {
+  // ignore 'if consteval'
+  markSkipped(S->getIfLoc(), getStart(Then));
+  propagateCounts(ParentCount, Then);
+
+  if (Else) {
+// ignore 'else '
+markSkipped(getEnd(Then), getEnd(Else));
+  }
+} else {
+  assert(S->isNonNegatedConsteval());
+  // ignore 'if consteval  [else]'
+  markSkipped(S->getIfLoc(), Else ? getStart(Else) : getEnd(Then));
+
+  if (Else)
+propagateCounts(ParentCount, Else);
+}
+  }
+
+  void CoverIfConstexpr(const IfStmt *S) {
+assert(S->isConstexpr());
+
+// evaluate constant condition...
+const auto *E = dyn_cast(S->getCond());
+assert(E != nullptr);
+const bool isTrue = E->getResultAsAPSInt().getExtValue();
+
+extendRegion(S);
+
+const auto *Init = S->getInit();
+const auto *Then = S->getThen();
+const auto *Else = S->getElse();
+
+// I'm using 'propagateCounts' later as new region is better and allows me
+// to properly calculate line coverage in llvm-cov utility
+const Counter ParentCount = getRegion().getCounter();
+
+// ignore 'if constexpr ('
+SourceLocation startOfSkipped = S->getIfLoc();
+
+if (Init) {
+  // don't mark initialisation as ignored
+  markSkipped(startOfSkipped, getStart(Init));
+  propagateCounts(ParentCount, Init);
+  // ignore after initialisation: '; )'...
+  startOfSkipped = getEnd(Init);
+}
+
+if (isTrue) {
+  // ignore ')'
+  markSkipped(startOfSkipped, getStart(Then));
+  propagateCounts(ParentCount, Then);
+
+  if (Else)
+// ignore 'else '
+markSkipped(getEnd(Then), getEnd(Else));
+} else {
+  // ignore ')  [else]'
+  markSkipped(startOfSkipped, Else ? getStart(Else) : getEnd(Then));
+
+  if (Else) {

ornata wrote:

style nit: LLVM prefers omitting braces on single-line `if`/`while`/`for`

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' (PR #78033)

2024-01-22 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= ,
Hana =?utf-8?q?Dusíková?= 
Message-ID:
In-Reply-To: 



@@ -1700,43 +1776,116 @@ struct CounterCoverageMappingBuilder
 Visit(S->getSubStmt());
   }
 
+  void CoverIfConsteval(const IfStmt *S) {

ornata wrote:

nit: LLVM style guidelines say function names ought to be camel case, starting 
with a lower-case letter

https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-16 Thread Jessica Paquette via cfe-commits


@@ -676,41 +679,25 @@ struct MCDCCoverageBuilder {
 return E->getOpcode() == BO_LAnd;
   }
 
-  /// Push an ID onto the corresponding RHS stack.
-  void pushRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-rhs.push_back(CondIDs[CodeGenFunction::stripCond(E->getRHS())]);
-  }
-
-  /// Pop an ID from the corresponding RHS stack.
-  void popRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-if (!rhs.empty())
-  rhs.pop_back();
-  }
-
-  /// If the expected ID is on top, pop it off the corresponding RHS stack.
-  void popRHSifTop(const BinaryOperator *E) {
-if (!OrRHS.empty() && CondIDs[E] == OrRHS.back())
-  OrRHS.pop_back();
-else if (!AndRHS.empty() && CondIDs[E] == AndRHS.back())
-  AndRHS.pop_back();
-  }
-
 public:
   MCDCCoverageBuilder(CodeGenModule &CGM,
   llvm::DenseMap &CondIDMap,
   llvm::DenseMap &MCDCBitmapMap)
-  : CGM(CGM), CondIDs(CondIDMap), MCDCBitmapMap(MCDCBitmapMap) {}
+  : CGM(CGM), DecisionStack(1), CondIDs(CondIDMap),
+MCDCBitmapMap(MCDCBitmapMap) {}
 
-  /// Return the ID of the RHS of the next, upper nest-level logical-OR.
-  MCDCConditionID getNextLOrCondID() const {
-return OrRHS.empty() ? 0 : OrRHS.back();
-  }
+  /// Return whether the control flow map is not presently being built. This
+  /// can be used to determine whether the flow is at the root node of an

ornata wrote:

I find the comment here kind of confusing.

I'd assume that `isIdle() == !isBuilding()` from this comment.

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-16 Thread Jessica Paquette via cfe-commits


@@ -676,41 +679,25 @@ struct MCDCCoverageBuilder {
 return E->getOpcode() == BO_LAnd;
   }
 
-  /// Push an ID onto the corresponding RHS stack.
-  void pushRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-rhs.push_back(CondIDs[CodeGenFunction::stripCond(E->getRHS())]);
-  }
-
-  /// Pop an ID from the corresponding RHS stack.
-  void popRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-if (!rhs.empty())
-  rhs.pop_back();
-  }
-
-  /// If the expected ID is on top, pop it off the corresponding RHS stack.
-  void popRHSifTop(const BinaryOperator *E) {
-if (!OrRHS.empty() && CondIDs[E] == OrRHS.back())
-  OrRHS.pop_back();
-else if (!AndRHS.empty() && CondIDs[E] == AndRHS.back())
-  AndRHS.pop_back();
-  }
-
 public:
   MCDCCoverageBuilder(CodeGenModule &CGM,
   llvm::DenseMap &CondIDMap,
   llvm::DenseMap &MCDCBitmapMap)
-  : CGM(CGM), CondIDs(CondIDMap), MCDCBitmapMap(MCDCBitmapMap) {}
+  : CGM(CGM), DecisionStack(1), CondIDs(CondIDMap),
+MCDCBitmapMap(MCDCBitmapMap) {}
 
-  /// Return the ID of the RHS of the next, upper nest-level logical-OR.
-  MCDCConditionID getNextLOrCondID() const {
-return OrRHS.empty() ? 0 : OrRHS.back();
-  }
+  /// Return whether the control flow map is not presently being built. This
+  /// can be used to determine whether the flow is at the root node of an
+  /// expression if that expression is mapped.
+  bool isIdle() { return (NextID == 1 && !NotMapped); }

ornata wrote:

these can be `const`

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-16 Thread Jessica Paquette via cfe-commits


@@ -676,41 +679,25 @@ struct MCDCCoverageBuilder {
 return E->getOpcode() == BO_LAnd;
   }
 
-  /// Push an ID onto the corresponding RHS stack.
-  void pushRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-rhs.push_back(CondIDs[CodeGenFunction::stripCond(E->getRHS())]);
-  }
-
-  /// Pop an ID from the corresponding RHS stack.
-  void popRHS(const BinaryOperator *E) {
-llvm::SmallVector &rhs = isLAnd(E) ? AndRHS : OrRHS;
-if (!rhs.empty())
-  rhs.pop_back();
-  }
-
-  /// If the expected ID is on top, pop it off the corresponding RHS stack.
-  void popRHSifTop(const BinaryOperator *E) {
-if (!OrRHS.empty() && CondIDs[E] == OrRHS.back())
-  OrRHS.pop_back();
-else if (!AndRHS.empty() && CondIDs[E] == AndRHS.back())
-  AndRHS.pop_back();
-  }
-
 public:
   MCDCCoverageBuilder(CodeGenModule &CGM,
   llvm::DenseMap &CondIDMap,
   llvm::DenseMap &MCDCBitmapMap)
-  : CGM(CGM), CondIDs(CondIDMap), MCDCBitmapMap(MCDCBitmapMap) {}
+  : CGM(CGM), DecisionStack(1), CondIDs(CondIDMap),

ornata wrote:

IIUC `DecisionStack(1)` is supposed to be a sentinel value.

Would it be possible to define a `DecisionStackSentinel` variable equal to 1, 
so we can document it?

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-16 Thread Jessica Paquette via cfe-commits


@@ -722,6 +709,12 @@ struct MCDCCoverageBuilder {
   return I->second;
   }
 
+  /// Return the LHS Decision ({0,0} if not set).
+  const DecisionIDPair &back() {
+assert(DecisionStack.size() >= 1);

ornata wrote:

This assert should be unnecessary. `SmallVector` already asserts for you:

```
  reference back() {
assert(!empty());
return end()[-1];
  }
  const_reference back() const {
assert(!empty());
return end()[-1];
  }
```

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-15 Thread Jessica Paquette via cfe-commits


@@ -298,7 +298,7 @@ struct CounterMappingRegion {
unsigned ExpandedFileID, unsigned LineStart,
unsigned ColumnStart, unsigned LineEnd,
unsigned ColumnEnd, RegionKind Kind)
-  : MCDCParams(MCDCParams), ExpandedFileID(ExpandedFileID),
+  : MCDCParams(MCDCParams), FileID(FileID), ExpandedFileID(ExpandedFileID),

ornata wrote:

Is this line the only bit necessary for the fix? Or are the other changes 
necessary?

If this is all that is necessary, then I would recommend splitting this into 
two patches.

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-15 Thread Jessica Paquette via cfe-commits


@@ -722,6 +739,24 @@ struct MCDCCoverageBuilder {
   return I->second;
   }
 
+  /// Return the ID of the next condition when the given condition is True.
+  MCDCConditionID getNextIfTrueCondID(const Expr *Cond) const {
+auto I = TrueCondIDs.find(CodeGenFunction::stripCond(Cond));
+if (I == TrueCondIDs.end())
+  return 0;

ornata wrote:

Should document that this returns 0 when the condition is not found.

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [llvm] [clang][CoverageMapping] Refactor when setting MC/DC True/False (PR #78202)

2024-01-15 Thread Jessica Paquette via cfe-commits


@@ -746,21 +781,52 @@ struct MCDCCoverageBuilder {
 // assign that ID to its LHS node.  Its RHS will receive a new ID.
 if (CondIDs.contains(CodeGenFunction::stripCond(E))) {
   // If Stmt has an ID, assign its ID to LHS
-  CondIDs[CodeGenFunction::stripCond(E->getLHS())] = CondIDs[E];
+  setCondID(E->getLHS(), getCondID(E));
 
   // Since the operator's LHS assumes the operator's same ID, pop the
   // operator from the RHS stack so that if LHS short-circuits, it won't be
   // incorrectly re-used as the node executed next.
   popRHSifTop(E);
 } else {
   // Otherwise, assign ID+1 to LHS.
-  CondIDs[CodeGenFunction::stripCond(E->getLHS())] = NextID++;
+  setCondID(E->getLHS(), NextID++);
 }
 
 // Assign ID+1 to RHS.
-CondIDs[CodeGenFunction::stripCond(E->getRHS())] = NextID++;
+setCondID(E->getRHS(), NextID++);
+
+// Assign the True and False condition IDs for the LHS and RHS.
+if (isLAnd(E)) {

ornata wrote:

I feel like it'd be easier to follow this if it was factored out into two 
functions. E.g.

```
if (isLAnd(E))
assignCondIDsForLAnd(E);
else
   assignCondIDsForLOr(E);
pushRHS(E);
```

https://github.com/llvm/llvm-project/pull/78202
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[llvm] [clang] [coverage] skipping code coverage for 'if constexpr' and 'if consteval' [WIP] (PR #78033)

2024-01-14 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusíková?= 
Message-ID:
In-Reply-To: 


ornata wrote:

I think that skipping whitespace-only regions in llvm-cov intuitively makes 
sense. In general, showing coverage information about empty spaces doesn't give 
the user any useful information.

I think I'd do that part in a separate patch though.

https://github.com/llvm/llvm-project/pull/78033
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= 
Message-ID:
In-Reply-To: 


https://github.com/ornata approved this pull request.


https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits
Hana =?utf-8?q?Dusi=CC=81kova=CC=81?= 
Message-ID:
In-Reply-To: 


ornata wrote:

LGTM

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -98,3 +104,49 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0

ornata wrote:

Maybe `check_if_consteval_with_discarded_branch` would be a better name?

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -98,3 +104,49 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0
+  if consteval {
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> 
[[@LINE+1]]:4 = #1
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 
-> [[@LINE]]:10 = #0
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = #0
+  }
+  return i; // CHECK-NEXT: [[@LINE]]:3 -> 
[[@LINE]]:11 = (#0 + #1)
+}
+
+// CHECK-LABEL: _Z16check_constevalBi:
+constexpr int check_constevalB(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0
+  if !consteval {
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:17 -> 
[[@LINE+1]]:4 = #0
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 
-> [[@LINE]]:10 = 0
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = 0
+  }
+  return i;
+}
+
+// CHECK-LABEL: _Z16check_constevalCi:
+constexpr int check_constevalC(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0

ornata wrote:

Similarly, this test and the below one could just be called 
`check_if_consteval` and `check_if_not_consteval`

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -23,19 +23,29 @@ void foo() {// CHECK-NEXT: Gap,File 0, 
[[@LINE+1]]:21 -> [[@
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 
= #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 
= #1
 
-// FIXME: Do not generate coverage for discarded branches in if consteval and 
if constexpr statements
-constexpr int check_consteval(int i) {
-if consteval {
-  i++;
-}
-if !consteval {
-  i++;
-}
-if consteval {
-return 42;
-} else {
-return i;
-}
+// FIXME: Do not generate coverage for discarded branches in if constexpr
+// CHECK-LABEL: _Z16check_constexprAi:
+int check_constexprA(int i) {   // CHECK-NEXT: [[@LINE]]:29 -> {{[0-9]+}}:2 = 
#0
+// CHECK-NEXT: [[@LINE+2]]:16 -> 
[[@LINE+2]]:20 = #0
+// CHECK-NEXT: Branch,File 0, [[@LINE+1]]:16 
-> [[@LINE+1]]:20 = 0, 0
+  if constexpr(true) {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:21 -> 
[[@LINE]]:22 = #1
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:22 -> [[@LINE+1]]:4 
= #1
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 -> 
[[@LINE]]:10 = (#0 - #1)
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> [[@LINE+1]]:4 
= (#0 - #1)
+  }
+  return i;
+}
+
+// CHECK-LABEL: _Z16check_constexprBi:
+int check_constexprB(int i) {   // CHECK-NEXT: [[@LINE]]:29 -> {{[0-9]+}}:2 = 
#0

ornata wrote:

Similarly, this would be better-named something like `check_if_constexpr_false`

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -98,3 +104,49 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0
+  if consteval {
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> 
[[@LINE+1]]:4 = #1
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 
-> [[@LINE]]:10 = #0
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = #0
+  }
+  return i; // CHECK-NEXT: [[@LINE]]:3 -> 
[[@LINE]]:11 = (#0 + #1)
+}
+
+// CHECK-LABEL: _Z16check_constevalBi:
+constexpr int check_constevalB(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0

ornata wrote:

Also does this one need a FIXME like in the above test?

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -7741,6 +7745,10 @@ TreeTransform::TransformIfStmt(IfStmt *S) {
 Else = getDerived().TransformStmt(S->getElse());
 if (Else.isInvalid())
   return StmtError();
+  } else if (S->getElse() && ConstexprConditionValue &&

ornata wrote:

Could you add a comment similar to the one in the `else` above?

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -23,19 +23,29 @@ void foo() {// CHECK-NEXT: Gap,File 0, 
[[@LINE+1]]:21 -> [[@
 }   // CHECK-NEXT: [[@LINE-2]]:9 -> [[@LINE-1]]:5 
= #1
 // CHECK-NEXT: [[@LINE-2]]:5 -> [[@LINE-2]]:8 
= #1
 
-// FIXME: Do not generate coverage for discarded branches in if consteval and 
if constexpr statements
-constexpr int check_consteval(int i) {
-if consteval {
-  i++;
-}
-if !consteval {
-  i++;
-}
-if consteval {
-return 42;
-} else {
-return i;
-}
+// FIXME: Do not generate coverage for discarded branches in if constexpr
+// CHECK-LABEL: _Z16check_constexprAi:
+int check_constexprA(int i) {   // CHECK-NEXT: [[@LINE]]:29 -> {{[0-9]+}}:2 = 
#0

ornata wrote:

I think this test should be named `check_if_constexpr_true` or something similar

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -98,3 +104,49 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0
+  if consteval {
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> 
[[@LINE+1]]:4 = #1
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 
-> [[@LINE]]:10 = #0
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = #0

ornata wrote:

Specifically, it's the `else` region that shouldn't generate any coverage 
mapping data, right?

It'd be good to mention that in the `FIXME`; something like

> FIXME: Do not generate coverage for discarded branches in if consteval. The 
> else case is discarded and should not be covered.

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits

https://github.com/ornata commented:

Added some comments, mostly nits on the test.

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -7732,7 +7732,11 @@ TreeTransform::TransformIfStmt(IfStmt *S) {
 if (Then.isInvalid())
   return StmtError();
   } else {
-Then = new (getSema().Context) NullStmt(S->getThen()->getBeginLoc());
+// Discarded branch is replaced with empty CompoundStmt so we can keep
+// proper source location for start and end of original branch, so
+// subsequent transformations like CoverageMapping work properly

ornata wrote:

In the test, you mention that we're generating coverage for the discarded 
regions. Is the reason that we're generating a `CompoundStmt` here? Could this 
use a `FIXME`?

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits


@@ -98,3 +104,49 @@ int main() {// CHECK: File 0, 
[[@LINE]]:12 -> {{[0-9]+}}:2 =
 void ternary() {
   true ? FOO : FOO; // CHECK-NOT: Gap,{{.*}}, [[@LINE]]:8 ->
 }
+
+// FIXME: Do not generate coverage for discarded branches in if consteval
+// GH-57377
+// CHECK-LABEL: _Z16check_constevalAi:
+constexpr int check_constevalA(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0
+  if consteval {
+i *= 3; // CHECK-NEXT: [[@LINE-1]]:16 -> 
[[@LINE+1]]:4 = #1
+  } else {  // CHECK-NEXT: Gap,File 0, [[@LINE]]:4 
-> [[@LINE]]:10 = #0
+i *= 5; // CHECK-NEXT: [[@LINE-1]]:10 -> 
[[@LINE+1]]:4 = #0
+  }
+  return i; // CHECK-NEXT: [[@LINE]]:3 -> 
[[@LINE]]:11 = (#0 + #1)
+}
+
+// CHECK-LABEL: _Z16check_constevalBi:
+constexpr int check_constevalB(int i) { // CHECK-NEXT: [[@LINE]]:39 -> 
{{[0-9]+}}:2 = #0

ornata wrote:

Maybe `check_if_not_consteval_with_discarded_branch` would be a more 
descriptive name?

https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][coverage] fixing "if constexpr" and "if consteval" coverage report (PR #77214)

2024-01-08 Thread Jessica Paquette via cfe-commits

https://github.com/ornata edited https://github.com/llvm/llvm-project/pull/77214
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 17cfd2e - [profiling] Improve error message for raw profile header mismatches

2023-04-27 Thread Jessica Paquette via cfe-commits

Author: Jessica Paquette
Date: 2023-04-27T14:51:38-07:00
New Revision: 17cfd2e025cb3aa929ad219c6ed0974d6198bf5b

URL: 
https://github.com/llvm/llvm-project/commit/17cfd2e025cb3aa929ad219c6ed0974d6198bf5b
DIFF: 
https://github.com/llvm/llvm-project/commit/17cfd2e025cb3aa929ad219c6ed0974d6198bf5b.diff

LOG: [profiling] Improve error message for raw profile header mismatches

When a user uses a mismatched clang + llvm-profdata, they didn't get a very
informative error message. It would just say "unsupported version".

As a result, users are often confused as to what they are supposed to do and
tend to assume that it's a bug in the profiling runtime.

This patch improves the error message by:

- Adding a new class of error (`raw_profile_version_mismatch`) to make it clear
  that, specifically, the *raw profile* version is unsupported because of a
  tool mismatch.

- Adding an error message that tells the user which raw profile version was
  encountered, which version was expected, and instructs them to align their
  tool versions.

To support this, this patch also updates `InstrProfError::take` to also
propagate the optional error message.

Differential Revision: https://reviews.llvm.org/D149361

Added: 
llvm/test/tools/llvm-profdata/mismatched-raw-profile-header.test

Modified: 
clang/lib/CodeGen/CodeGenPGO.cpp
llvm/include/llvm/ProfileData/InstrProf.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/ProfileData/InstrProfReader.cpp
llvm/tools/llvm-profdata/llvm-profdata.cpp
llvm/unittests/ProfileData/InstrProfTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenPGO.cpp 
b/clang/lib/CodeGen/CodeGenPGO.cpp
index 15a3d74666ca2..b80317529b72b 100644
--- a/clang/lib/CodeGen/CodeGenPGO.cpp
+++ b/clang/lib/CodeGen/CodeGenPGO.cpp
@@ -1036,7 +1036,7 @@ void 
CodeGenPGO::loadRegionCounts(llvm::IndexedInstrProfReader *PGOReader,
   llvm::Expected RecordExpected =
   PGOReader->getInstrProfRecord(FuncName, FunctionHash);
   if (auto E = RecordExpected.takeError()) {
-auto IPE = llvm::InstrProfError::take(std::move(E));
+auto IPE = std::get<0>(llvm::InstrProfError::take(std::move(E)));
 if (IPE == llvm::instrprof_error::unknown_function)
   CGM.getPGOStats().addMissing(IsInMainFile);
 else if (IPE == llvm::instrprof_error::hash_mismatch)

diff  --git a/llvm/include/llvm/ProfileData/InstrProf.h 
b/llvm/include/llvm/ProfileData/InstrProf.h
index 2a9da23f4f02f..ef01070832dee 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -330,7 +330,8 @@ enum class instrprof_error {
   compress_failed,
   uncompress_failed,
   empty_raw_profile,
-  zlib_unavailable
+  zlib_unavailable,
+  raw_profile_version_mismatch
 };
 
 /// An ordered list of functions identified by their NameRef found in
@@ -362,15 +363,18 @@ class InstrProfError : public ErrorInfo {
   instrprof_error get() const { return Err; }
   const std::string &getMessage() const { return Msg; }
 
-  /// Consume an Error and return the raw enum value contained within it. The
-  /// Error must either be a success value, or contain a single InstrProfError.
-  static instrprof_error take(Error E) {
+  /// Consume an Error and return the raw enum value contained within it, and
+  /// the optional error message. The Error must either be a success value, or
+  /// contain a single InstrProfError.
+  static std::pair take(Error E) {
 auto Err = instrprof_error::success;
-handleAllErrors(std::move(E), [&Err](const InstrProfError &IPE) {
+std::string Msg = "";
+handleAllErrors(std::move(E), [&Err, &Msg](const InstrProfError &IPE) {
   assert(Err == instrprof_error::success && "Multiple errors encountered");
   Err = IPE.get();
+  Msg = IPE.getMessage();
 });
-return Err;
+return {Err, Msg};
   }
 
   static char ID;

diff  --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp 
b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index d6aec276838e6..849ee80bfaa33 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -249,7 +249,7 @@ Error CoverageMapping::loadFunctionRecord(
   std::vector Counts;
   if (Error E = ProfileReader.getFunctionCounts(Record.FunctionName,
 Record.FunctionHash, Counts)) {
-instrprof_error IPE = InstrProfError::take(std::move(E));
+instrprof_error IPE = std::get<0>(InstrProfError::take(std::move(E)));
 if (IPE == instrprof_error::hash_mismatch) {
   FuncHashMismatches.emplace_back(std::string(Record.FunctionName),
   Record.FunctionHash);

diff  --git a/llvm/lib/ProfileData/InstrProf.cpp 
b/llvm/lib/ProfileData/InstrProf.cpp
index d8dbcf1ebbe9a..1dd1ce4b3e50a 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp

[clang] b9e57e0 - Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to check entire function calls, rather than each ExplodedNode in it"

2021-09-03 Thread Jessica Paquette via cfe-commits

Author: Jessica Paquette
Date: 2021-09-03T10:28:07-07:00
New Revision: b9e57e030560fef9ddc51caca8bacfefccdf8a62

URL: 
https://github.com/llvm/llvm-project/commit/b9e57e030560fef9ddc51caca8bacfefccdf8a62
DIFF: 
https://github.com/llvm/llvm-project/commit/b9e57e030560fef9ddc51caca8bacfefccdf8a62.diff

LOG: Revert "[analyzer][NFCI] Allow clients of NoStateChangeFuncVisitor to 
check entire function calls, rather than each ExplodedNode in it"

This reverts commit a375bfb5b729e0f3ca8d5e001f423fa89e74de87.

This was causing a bot to crash:

https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/23380/

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/CallEventTest.cpp
clang/unittests/StaticAnalyzer/CheckerRegistration.h
clang/unittests/StaticAnalyzer/FalsePositiveRefutationBRVisitorTest.cpp
clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp

Removed: 
clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp



diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h 
b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index c42521376af92..139b0dcd51704 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -633,9 +633,6 @@ class CXXConstructorCall;
 /// Descendants can define what a "state change is", like a change of value
 /// to a memory region, liveness, etc. For function calls where the state did
 /// not change as defined, a custom note may be constructed.
-///
-/// For a minimal example, check out
-/// clang/unittests/StaticAnalyzer/NoStateChangeFuncVisitorTest.cpp.
 class NoStateChangeFuncVisitor : public BugReporterVisitor {
 private:
   /// Frames modifying the state as defined in \c wasModifiedBeforeCallExit.
@@ -646,8 +643,6 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// many times (going up the path for each node and checking whether the
   /// region was written into) we instead lazily compute the stack frames
   /// along the path.
-  // TODO: Can't we just use a map instead? This is likely not as cheap as it
-  // makes the code 
diff icult to read.
   llvm::SmallPtrSet FramesModifying;
   llvm::SmallPtrSet FramesModifyingCalculated;
 
@@ -656,8 +651,6 @@ class NoStateChangeFuncVisitor : public BugReporterVisitor {
   /// The calculation is cached in FramesModifying.
   bool isModifiedInFrame(const ExplodedNode *CallExitBeginN);
 
-  void markFrameAsModifying(const StackFrameContext *SCtx);
-
   /// Write to \c FramesModifying all stack frames along the path in the 
current
   /// stack frame which modifies the state.
   void findModifyingFrames(const ExplodedNode *const CallExitBeginN);
@@ -665,37 +658,11 @@ class NoStateChangeFuncVisitor : public 
BugReporterVisitor {
 protected:
   bugreporter::TrackingKind TKind;
 
-  /// \return Whether the state was modified from the current node, \p CurrN, 
to
-  /// the end of the stack frame, at \p CallExitBeginN. \p CurrN and
-  /// \p CallExitBeginN are always in the same stack frame.
-  /// Clients should override this callback when a state change is important
-  /// not only on the entire function call, but inside of it as well.
-  /// Example: we may want to leave a note about the lack of locking/unlocking
-  /// on a particular mutex, but not if inside the function its state was
-  /// changed, but also restored. wasModifiedInFunction() wouldn't know of this
-  /// change.
-  virtual bool wasModifiedBeforeCallExit(const ExplodedNode *CurrN,
- const ExplodedNode *CallExitBeginN) {
-return false;
-  }
-
-  /// \return Whether the state was modified in the inlined function call in
-  /// between \p CallEnterN and \p CallExitEndN. Mind that the stack frame
-  /// retrieved from a CallEnterN and CallExitEndN is the *caller's* stack
-  /// frame! The inlined function's stack should be retrieved from either the
-  /// immediate successor to \p CallEnterN or immediate predecessor to
-  /// \p CallExitEndN.
-  /// Clients should override this function if a state changes local to the
-  /// inlined function are not interesting, only the change occuring as a
-  /// result of it.
-  /// Example: we want to leave a not about a leaked resource object not being
-  /// deallocated / its ownership changed inside a function, and we don't care
-  /// if it was assigned to a local variable (its change in ownership is
-  /// inconsequential).
-  virtual bool wasModifiedInFunction(const ExplodedNode *CallEnterN,
- const Ex

r357031 - Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

2019-03-26 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Tue Mar 26 14:22:42 2019
New Revision: 357031

URL: http://llvm.org/viewvc/llvm-project?rev=357031&view=rev
Log:
Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

Since AArch64 has default outlining behaviour, we need to make sure that
-mno-outline is actually passed along to the linker in this case. Otherwise,
it will run by default on minsize functions even when -mno-outline is specified.

Also fix the darwin-ld test for this, which wasn't actually doing anything.

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Tue Mar 26 14:22:42 2019
@@ -494,14 +494,23 @@ void darwin::Linker::ConstructJob(Compil
   }
 
   // Propagate the -moutline flag to the linker in LTO.
-  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
-if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-machine-outliner");
+  if (Arg *A =
+  Args.getLastArg(options::OPT_moutline, options::OPT_mno_outline)) {
+if (A->getOption().matches(options::OPT_moutline)) {
+  if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-machine-outliner");
 
-  // Outline from linkonceodr functions by default in LTO.
+// Outline from linkonceodr functions by default in LTO.
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-linkonceodr-outlining");
+  }
+} else {
+  // Disable all outlining behaviour if we have mno-outline. We need to do
+  // this explicitly, because targets which support default outlining will
+  // try to do work if we don't.
   CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-linkonceodr-outlining");
+  CmdArgs.push_back("-enable-machine-outliner=never");
 }
   }
 

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Tue Mar 26 14:22:42 2019
@@ -372,5 +372,11 @@
 // Check that we can pass the outliner down to the linker.
 // RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
 // RUN:   %clang -target arm64-apple-darwin -moutline -### %t.o 2> %t.log
-// MOUTLINE: ld
+// RUN: FileCheck -check-prefix=MOUTLINE %s < %t.log
+// MOUTLINE: {{ld(.exe)?"}}
 // MOUTLINE-SAME: "-mllvm" "-enable-machine-outliner" "-mllvm" 
"-enable-linkonceodr-outlining"
+// RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
+// RUN:   %clang -target arm64-apple-darwin -mno-outline -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=MNO_OUTLINE %s < %t.log
+// MNO_OUTLINE: {{ld(.exe)?"}}
+// MNO_OUTLINE-SAME: "-mllvm" "-enable-machine-outliner=never"


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


r336471 - [MachineOutliner] Properly pass -moutline along to the toolchain

2018-07-06 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Fri Jul  6 15:24:56 2018
New Revision: 336471

URL: http://llvm.org/viewvc/llvm-project?rev=336471&view=rev
Log:
[MachineOutliner] Properly pass -moutline along to the toolchain

This moves the LTO-specific code for outlining from ToolChains/Clang.cpp to
ToolChains/Darwin.cpp. Passing -mllvm flags isn't sufficient for making sure
that the specified pass will actually run in LTO. This makes sure that when
-moutline is passed, the MachineOutliner will actually be added to the LTO
pass pipeline as expected.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/aarch64-outliner.c
cfe/trunk/test/Driver/darwin-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=336471&r1=336470&r2=336471&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Jul  6 15:24:56 2018
@@ -4757,16 +4757,8 @@ void Clang::ConstructJob(Compilation &C,
   if (Triple.getArch() != llvm::Triple::aarch64) {
 D.Diag(diag::warn_drv_moutline_unsupported_opt) << 
Triple.getArchName();
   } else {
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back("-enable-machine-outliner");
-
-// The outliner shouldn't compete with linkers that dedupe linkonceodr
-// functions in LTO. Enable that behaviour by default when compiling 
with
-// LTO.
-if (getToolChain().getDriver().isUsingLTO()) {
   CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-linkonceodr-outlining");
-}
+  CmdArgs.push_back("-enable-machine-outliner");
   }
 } else {
   // Disable all outlining behaviour.

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=336471&r1=336470&r2=336471&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Fri Jul  6 15:24:56 2018
@@ -479,6 +479,18 @@ void darwin::Linker::ConstructJob(Compil
 }
   }
 
+  // Propagate the -moutline flag to the linker in LTO.
+  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
+if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-machine-outliner");
+
+  // Outline from linkonceodr functions by default in LTO.
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-linkonceodr-outlining");
+}
+  }
+
   // It seems that the 'e' option is completely ignored for dynamic executables
   // (the default), and with static executables, the last one wins, as 
expected.
   Args.AddAllArgs(CmdArgs, {options::OPT_d_Flag, options::OPT_s, 
options::OPT_t,

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=336471&r1=336470&r2=336471&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Fri Jul  6 15:24:56 2018
@@ -3,10 +3,6 @@
 // ON: "-mllvm" "-enable-machine-outliner"
 // RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF
 // OFF: "-mllvm" "-enable-machine-outliner=never"
-// RUN: %clang -target aarch64 -moutline -flto=thin -S %s -### 2>&1 | 
FileCheck %s -check-prefix=FLTO
-// FLTO: "-mllvm" "-enable-linkonceodr-outlining"
-// RUN: %clang -target aarch64 -moutline -flto=full -S %s -### 2>&1 | 
FileCheck %s -check-prefix=TLTO
-// TLTO: "-mllvm" "-enable-linkonceodr-outlining"
 // RUN: %clang -target x86_64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=WARN
 // WARN: warning: The 'x86_64' architecture does not support -moutline; flag 
ignored [-Woption-ignored]
 // WARN-NOT: "-mllvm" "-enable-machine-outliner"

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=336471&r1=336470&r2=336471&view=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Fri Jul  6 15:24:56 2018
@@ -371,3 +371,9 @@
 // RUN: %clang -target x86_64-apple-darwin12 -fprofile-instr-generate -### 
%t.o 2> %t.log
 // RUN: FileCheck -check-prefix=NO_PROFILE_EXPORT %s < %t.log
 // NO_PROFILE_EXPORT-NOT: "-exported_symbol"
+//
+// Check that we can pass the outliner down to the linker.
+// RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
+// RUN:   %clang -target arm64-apple-darwin -moutline -### %t.o 2> %t.log
+// MOUTLI

r336001 - [MachineOutliner] Make -mno-outline use -enable-machine-outliner=never

2018-06-29 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Fri Jun 29 11:06:10 2018
New Revision: 336001

URL: http://llvm.org/viewvc/llvm-project?rev=336001&view=rev
Log:
[MachineOutliner] Make -mno-outline use -enable-machine-outliner=never

This updates -mno-outline so that it passes -enable-machine-outliner=never
instead of nothing. This puts it in sync with the behaviour in llc and
other tools.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=336001&r1=336000&r2=336001&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Jun 29 11:06:10 2018
@@ -4797,23 +4797,30 @@ void Clang::ConstructJob(Compilation &C,
options::OPT_fno_complete_member_pointers, false))
 CmdArgs.push_back("-fcomplete-member-pointers");
 
-  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
-// We only support -moutline in AArch64 right now. If we're not compiling
-// for AArch64, emit a warning and ignore the flag. Otherwise, add the
-// proper mllvm flags.
-if (Triple.getArch() != llvm::Triple::aarch64) {
-  D.Diag(diag::warn_drv_moutline_unsupported_opt) << Triple.getArchName();
-} else {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-machine-outliner");
-
-  // The outliner shouldn't compete with linkers that dedupe linkonceodr
-  // functions in LTO. Enable that behaviour by default when compiling with
-  // LTO.
-  if (getToolChain().getDriver().isUsingLTO()) {
+  if (Arg *A = Args.getLastArg(options::OPT_moutline,
+   options::OPT_mno_outline)) {
+if (A->getOption().matches(options::OPT_moutline)) {
+  // We only support -moutline in AArch64 right now. If we're not compiling
+  // for AArch64, emit a warning and ignore the flag. Otherwise, add the
+  // proper mllvm flags.
+  if (Triple.getArch() != llvm::Triple::aarch64) {
+D.Diag(diag::warn_drv_moutline_unsupported_opt) << 
Triple.getArchName();
+  } else {
 CmdArgs.push_back("-mllvm");
-CmdArgs.push_back("-enable-linkonceodr-outlining");
+CmdArgs.push_back("-enable-machine-outliner");
+
+// The outliner shouldn't compete with linkers that dedupe linkonceodr
+// functions in LTO. Enable that behaviour by default when compiling 
with
+// LTO.
+if (getToolChain().getDriver().isUsingLTO()) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-linkonceodr-outlining");
+}
   }
+} else {
+  // Disable all outlining behaviour.
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-machine-outliner=never");
 }
   }
 

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=336001&r1=336000&r2=336001&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Fri Jun 29 11:06:10 2018
@@ -2,7 +2,7 @@
 // RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=ON
 // ON: "-mllvm" "-enable-machine-outliner"
 // RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF
-// OFF-NOT: "-mllvm" "-enable-machine-outliner"
+// OFF: "-mllvm" "-enable-machine-outliner=never"
 // RUN: %clang -target aarch64 -moutline -flto=thin -S %s -### 2>&1 | 
FileCheck %s -check-prefix=FLTO
 // FLTO: "-mllvm" "-enable-linkonceodr-outlining"
 // RUN: %clang -target aarch64 -moutline -flto=full -S %s -### 2>&1 | 
FileCheck %s -check-prefix=TLTO


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


r335672 - [MachineOutliner] Emit a warning when using -moutline on unsupported targets

2018-06-26 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Tue Jun 26 15:09:48 2018
New Revision: 335672

URL: http://llvm.org/viewvc/llvm-project?rev=335672&view=rev
Log:
[MachineOutliner] Emit a warning when using -moutline on unsupported targets

Instead of just saying "flag unused", we should tell the user that the
outliner isn't (at least officially) supported for some given architecture.

This adds a warning that will state something like

The 'blah' architecture does not support -moutline; flag ignored

when we call -moutline with the 'blah' architecture.

Since the outliner is still mostly an AArch64 thing, any architecture
other than AArch64 will emit this warning.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=335672&r1=335671&r2=335672&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Tue Jun 26 15:09:48 
2018
@@ -385,4 +385,8 @@ def warn_drv_experimental_isel_incomplet
 def warn_drv_experimental_isel_incomplete_opt : Warning<
   "-fexperimental-isel support is incomplete for this architecture at the 
current optimization level">,
   InGroup;
+
+def warn_drv_moutline_unsupported_opt : Warning<
+  "The '%0' architecture does not support -moutline; flag ignored">,
+  InGroup;
 }

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=335672&r1=335671&r2=335672&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Jun 26 15:09:48 2018
@@ -1476,19 +1476,6 @@ void Clang::AddAArch64TargetArgs(const A
 else
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
-
-  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back("-enable-machine-outliner");
-
-// The outliner shouldn't compete with linkers that dedupe linkonceodr
-// functions in LTO. Enable that behaviour by default when compiling with
-// LTO.
-if (getToolChain().getDriver().isUsingLTO()) {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-linkonceodr-outlining");
-}
-  }
 }
 
 void Clang::AddMIPSTargetArgs(const ArgList &Args,
@@ -4814,6 +4801,26 @@ void Clang::ConstructJob(Compilation &C,
options::OPT_fno_complete_member_pointers, false))
 CmdArgs.push_back("-fcomplete-member-pointers");
 
+  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
+// We only support -moutline in AArch64 right now. If we're not compiling
+// for AArch64, emit a warning and ignore the flag. Otherwise, add the
+// proper mllvm flags.
+if (Triple.getArch() != llvm::Triple::aarch64) {
+  D.Diag(diag::warn_drv_moutline_unsupported_opt) << Triple.getArchName();
+} else {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-machine-outliner");
+
+  // The outliner shouldn't compete with linkers that dedupe linkonceodr
+  // functions in LTO. Enable that behaviour by default when compiling with
+  // LTO.
+  if (getToolChain().getDriver().isUsingLTO()) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-linkonceodr-outlining");
+  }
+}
+  }
+
   // Finally add the compile command to the compilation.
   if (Args.hasArg(options::OPT__SLASH_fallback) &&
   Output.getType() == types::TY_Object &&

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=335672&r1=335671&r2=335672&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue Jun 26 15:09:48 2018
@@ -7,3 +7,6 @@
 // FLTO: "-mllvm" "-enable-linkonceodr-outlining"
 // RUN: %clang -target aarch64 -moutline -flto=full -S %s -### 2>&1 | 
FileCheck %s -check-prefix=TLTO
 // TLTO: "-mllvm" "-enable-linkonceodr-outlining"
+// RUN: %clang -target x86_64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=WARN
+// WARN: warning: The 'x86_64' architecture does not support -moutline; flag 
ignored [-Woption-ignored]
+// WARN-NOT: "-mllvm" "-enable-machine-outliner"


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


r335549 - [MachineOutliner] NFC - simplify -moutline/-mno-outline logic

2018-06-25 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Mon Jun 25 16:20:18 2018
New Revision: 335549

URL: http://llvm.org/viewvc/llvm-project?rev=335549&view=rev
Log:
[MachineOutliner] NFC - simplify -moutline/-mno-outline logic

It's a bit cleaner to use `hasFlag` instead of nested ifs. This
just refactors the -moutline/-mno-outline logic to use that.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=335549&r1=335548&r2=335549&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 25 16:20:18 2018
@@ -1477,19 +1477,16 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_moutline,
-   options::OPT_mno_outline)) {
-if (A->getOption().matches(options::OPT_moutline)) {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-machine-outliner");
+  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-machine-outliner");
 
-  // The outliner shouldn't compete with linkers that dedupe linkonceodr
-  // functions in LTO. Enable that behaviour by default when compiling with
-  // LTO.
-  if (getToolChain().getDriver().isUsingLTO()) {
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back("-enable-linkonceodr-outlining");
-  }
+// The outliner shouldn't compete with linkers that dedupe linkonceodr
+// functions in LTO. Enable that behaviour by default when compiling with
+// LTO.
+if (getToolChain().getDriver().isUsingLTO()) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-linkonceodr-outlining");
 }
   }
 }


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


r335504 - [MachineOutliner] Outline from linkonceodrs by default in LTO when -moutline is passed

2018-06-25 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Mon Jun 25 10:36:05 2018
New Revision: 335504

URL: http://llvm.org/viewvc/llvm-project?rev=335504&view=rev
Log:
[MachineOutliner] Outline from linkonceodrs by default in LTO when -moutline is 
passed

Pass -enable-linkonceodr-outlining by default when LTO is enabled.

The outliner shouldn't compete with any sort of linker deduplication
on linkonceodr functions when LTO is enabled. Therefore, this behaviour
should be the default.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=335504&r1=335503&r2=335504&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 25 10:36:05 2018
@@ -1482,6 +1482,14 @@ void Clang::AddAArch64TargetArgs(const A
 if (A->getOption().matches(options::OPT_moutline)) {
   CmdArgs.push_back("-mllvm");
   CmdArgs.push_back("-enable-machine-outliner");
+
+  // The outliner shouldn't compete with linkers that dedupe linkonceodr
+  // functions in LTO. Enable that behaviour by default when compiling with
+  // LTO.
+  if (getToolChain().getDriver().isUsingLTO()) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-linkonceodr-outlining");
+  }
 }
   }
 }

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=335504&r1=335503&r2=335504&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Mon Jun 25 10:36:05 2018
@@ -3,3 +3,7 @@
 // ON: "-mllvm" "-enable-machine-outliner"
 // RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF
 // OFF-NOT: "-mllvm" "-enable-machine-outliner"
+// RUN: %clang -target aarch64 -moutline -flto=thin -S %s -### 2>&1 | 
FileCheck %s -check-prefix=FLTO
+// FLTO: "-mllvm" "-enable-linkonceodr-outlining"
+// RUN: %clang -target aarch64 -moutline -flto=full -S %s -### 2>&1 | 
FileCheck %s -check-prefix=TLTO
+// TLTO: "-mllvm" "-enable-linkonceodr-outlining"


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


r335503 - [MachineOutliner] Make last of -moutline/-mno-outline win

2018-06-25 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Mon Jun 25 10:27:51 2018
New Revision: 335503

URL: http://llvm.org/viewvc/llvm-project?rev=335503&view=rev
Log:
[MachineOutliner] Make last of -moutline/-mno-outline win

The expected behaviour of command-line flags to clang is to have
the last of -m(whatever) and -mno-(whatever) win. The outliner
didn't do that. This fixes that and updates the test.

Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=335503&r1=335502&r2=335503&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 25 10:27:51 2018
@@ -1477,10 +1477,12 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (!Args.hasArg(options::OPT_mno_outline) &&
-   Args.getLastArg(options::OPT_moutline)) {
-CmdArgs.push_back("-mllvm");
-CmdArgs.push_back("-enable-machine-outliner");
+  if (Arg *A = Args.getLastArg(options::OPT_moutline,
+   options::OPT_mno_outline)) {
+if (A->getOption().matches(options::OPT_moutline)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-enable-machine-outliner");
+}
   }
 }
 

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=335503&r1=335502&r2=335503&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Mon Jun 25 10:27:51 2018
@@ -1,9 +1,5 @@
 // REQUIRES: aarch64-registered-target
-
 // RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=ON
 // ON: "-mllvm" "-enable-machine-outliner"
-
-// RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF1
-// RUN: %clang -target aarch64 -mno-outline -moutline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF2
-// OFF1-NOT: "-mllvm" "-enable-machine-outliner"
-// OFF2-NOT: "-mllvm" "-enable-machine-outliner"
+// RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF
+// OFF-NOT: "-mllvm" "-enable-machine-outliner"


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


r331810 - Add a mno-outline flag to disable the MachineOutliner

2018-05-08 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Tue May  8 13:58:32 2018
New Revision: 331810

URL: http://llvm.org/viewvc/llvm-project?rev=331810&view=rev
Log:
Add a mno-outline flag to disable the MachineOutliner

Since we're working on turning the MachineOutliner by default under -Oz for
AArch64, it makes sense to have an -mno-outline flag available. This currently
doesn't do much (it basically just undoes -moutline).

When the MachineOutliner is on by default under AArch64, this flag should
set -mllvm -enable-machine-outliner=never.

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May  8 13:58:32 2018
@@ -1908,6 +1908,8 @@ def mms_bitfields : Flag<["-"], "mms-bit
   HelpText<"Set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def moutline : Flag<["-"], "moutline">, Group, 
Flags<[CC1Option]>,
 HelpText<"Enable function outlining (AArch64 only)">;
+def mno_outline : Flag<["-"], "mno-outline">, Group, 
Flags<[CC1Option]>,
+HelpText<"Disable function outlining (AArch64 only)">;
 def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
   HelpText<"Do not set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 13:58:32 2018
@@ -1485,7 +1485,8 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Args.getLastArg(options::OPT_moutline)) {
+  if (!Args.hasArg(options::OPT_mno_outline) &&
+   Args.getLastArg(options::OPT_moutline)) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-enable-machine-outliner");
   }

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331810&r1=331809&r2=331810&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May  8 13:58:32 2018
@@ -1,4 +1,9 @@
 // REQUIRES: aarch64-registered-target
 
-// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
-// CHECK: "-mllvm" "-enable-machine-outliner"
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s 
-check-prefix=ON
+// ON: "-mllvm" "-enable-machine-outliner"
+
+// RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF1
+// RUN: %clang -target aarch64 -mno-outline -moutline -S %s -### 2>&1 | 
FileCheck %s -check-prefix=OFF2
+// OFF1-NOT: "-mllvm" "-enable-machine-outliner"
+// OFF2-NOT: "-mllvm" "-enable-machine-outliner"


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


r331806 - Change -foutline to -moutline

2018-05-08 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Tue May  8 13:53:19 2018
New Revision: 331806

URL: http://llvm.org/viewvc/llvm-project?rev=331806&view=rev
Log:
Change -foutline to -moutline

Nitpicky, but the MachineOutliner is a machine-level pass, and so we should
reflect that by using "m" instead of "n".

Figured we should get this in before people get used to the letter f. :)

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/aarch64-outliner.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue May  8 13:53:19 2018
@@ -1317,8 +1317,6 @@ def fno_exceptions : Flag<["-"], "fno-ex
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, 
Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, 
Group, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group, 
Flags<[CC1Option]>;
-def foutline : Flag<["-"], "foutline">, Group, 
Flags<[CC1Option]>,
-HelpText<"Enable function outlining (AArch64 only)">;
 def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, 
Group,
   HelpText<"Disables the experimental global instruction selector">;
 def fno_experimental_new_pass_manager : Flag<["-"], 
"fno-experimental-new-pass-manager">,
@@ -1908,6 +1906,8 @@ def mmacos_version_min_EQ : Joined<["-"]
   Group, Alias;
 def mms_bitfields : Flag<["-"], "mms-bitfields">, Group, 
Flags<[CC1Option]>,
   HelpText<"Set the default structure layout to be compatible with the 
Microsoft compiler standard">;
+def moutline : Flag<["-"], "moutline">, Group, 
Flags<[CC1Option]>,
+HelpText<"Enable function outlining (AArch64 only)">;
 def mno_ms_bitfields : Flag<["-"], "mno-ms-bitfields">, Group,
   HelpText<"Do not set the default structure layout to be compatible with the 
Microsoft compiler standard">;
 def mstackrealign : Flag<["-"], "mstackrealign">, Group, 
Flags<[CC1Option]>,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue May  8 13:53:19 2018
@@ -1485,7 +1485,7 @@ void Clang::AddAArch64TargetArgs(const A
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Args.getLastArg(options::OPT_foutline)) {
+  if (Args.getLastArg(options::OPT_moutline)) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-enable-machine-outliner");
   }

Modified: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331806&r1=331805&r2=331806&view=diff
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (original)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Tue May  8 13:53:19 2018
@@ -1,4 +1,4 @@
 // REQUIRES: aarch64-registered-target
 
-// RUN: %clang -target aarch64 -foutline -S %s -### 2>&1 | FileCheck %s
+// RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s
 // CHECK: "-mllvm" "-enable-machine-outliner"


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


Re: r331370 - Add -foutline option to enable the MachineOutliner in AArch64

2018-05-02 Thread Jessica Paquette via cfe-commits
Hmm yeah, you’re right, especially considering D45916. It would be nice to have 
the desired behaviour just “fall out” of that patch. I’ll write a patch adding 
the flag.

- Jessica

> On May 2, 2018, at 2:48 PM, Friedman, Eli  wrote:
> 
> On 5/2/2018 9:42 AM, Jessica Paquette via cfe-commits wrote:
>> Author: paquette
>> Date: Wed May  2 09:42:51 2018
>> New Revision: 331370
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=331370&view=rev
>> Log:
>> Add -foutline option to enable the MachineOutliner in AArch64
> 
> It probably makes sense to also have a -fno-outline option.
> 
> -Eli
> 
>> 
>> Since we've been working on productizing the MachineOutliner in AArch64, it
>> makes sense to provide a more user-friendly way to enable it.
>> 
>> This allows users of AArch64 to enable the outliner using -foutline instead
>> of -mllvm -enable-machine-outliner. Other, less mature implementations (e.g,
>> x86-64) can still enable the pass using the -mllvm option.
>> 
>> Also add a test to make sure it works.
>> 
>> Added:
>> cfe/trunk/test/Driver/aarch64-outliner.c
>> Modified:
>> cfe/trunk/include/clang/Driver/Options.td
>> cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> 
>> Modified: cfe/trunk/include/clang/Driver/Options.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331370&r1=331369&r2=331370&view=diff
>> ==
>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/Options.td Wed May  2 09:42:51 2018
>> @@ -1317,6 +1317,8 @@ def fno_exceptions : Flag<["-"], "fno-ex
>>  def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, 
>> Flags<[CC1Option]>;
>>  def fno_inline_functions : Flag<["-"], "fno-inline-functions">, 
>> Group, Flags<[CC1Option]>;
>>  def fno_inline : Flag<["-"], "fno-inline">, Group, 
>> Flags<[CC1Option]>;
>> +def foutline : Flag<["-"], "foutline">, Group, 
>> Flags<[CC1Option]>,
>> +HelpText<"Enable function outlining (AArch64 only)">;
>>  def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, 
>> Group,
>>HelpText<"Disables the experimental global instruction selector">;
>>  def fno_experimental_new_pass_manager : Flag<["-"], 
>> "fno-experimental-new-pass-manager">,
>> 
>> Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331370&r1=331369&r2=331370&view=diff
>> ==
>> --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
>> +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed May  2 09:42:51 2018
>> @@ -1484,6 +1484,11 @@ void Clang::AddAArch64TargetArgs(const A
>>  else
>>CmdArgs.push_back("-aarch64-enable-global-merge=true");
>>}
>> +
>> +  if (Arg *A = Args.getLastArg(options::OPT_foutline)) {
>> +CmdArgs.push_back("-mllvm");
>> +CmdArgs.push_back("-enable-machine-outliner");
>> +  }
>>  }
>>void Clang::AddMIPSTargetArgs(const ArgList &Args,
>> 
>> Added: cfe/trunk/test/Driver/aarch64-outliner.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331370&view=auto
>> ==
>> --- cfe/trunk/test/Driver/aarch64-outliner.c (added)
>> +++ cfe/trunk/test/Driver/aarch64-outliner.c Wed May  2 09:42:51 2018
>> @@ -0,0 +1,4 @@
>> +// REQUIRES: aarch64-registered-target
>> +
>> +// RUN: %clang -target aarch64 -foutline -S %s -### 2>&1 | FileCheck %s
>> +// CHECK: "-mllvm" "-enable-machine-outliner"
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
> 
> -- 
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
> Foundation Collaborative Project
> 

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


r331370 - Add -foutline option to enable the MachineOutliner in AArch64

2018-05-02 Thread Jessica Paquette via cfe-commits
Author: paquette
Date: Wed May  2 09:42:51 2018
New Revision: 331370

URL: http://llvm.org/viewvc/llvm-project?rev=331370&view=rev
Log:
Add -foutline option to enable the MachineOutliner in AArch64

Since we've been working on productizing the MachineOutliner in AArch64, it
makes sense to provide a more user-friendly way to enable it.

This allows users of AArch64 to enable the outliner using -foutline instead
of -mllvm -enable-machine-outliner. Other, less mature implementations (e.g,
x86-64) can still enable the pass using the -mllvm option.

Also add a test to make sure it works.

Added:
cfe/trunk/test/Driver/aarch64-outliner.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=331370&r1=331369&r2=331370&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed May  2 09:42:51 2018
@@ -1317,6 +1317,8 @@ def fno_exceptions : Flag<["-"], "fno-ex
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, 
Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, 
Group, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group, 
Flags<[CC1Option]>;
+def foutline : Flag<["-"], "foutline">, Group, 
Flags<[CC1Option]>,
+HelpText<"Enable function outlining (AArch64 only)">;
 def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, 
Group,
   HelpText<"Disables the experimental global instruction selector">;
 def fno_experimental_new_pass_manager : Flag<["-"], 
"fno-experimental-new-pass-manager">,

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=331370&r1=331369&r2=331370&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Wed May  2 09:42:51 2018
@@ -1484,6 +1484,11 @@ void Clang::AddAArch64TargetArgs(const A
 else
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
+
+  if (Arg *A = Args.getLastArg(options::OPT_foutline)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-machine-outliner");
+  }
 }
 
 void Clang::AddMIPSTargetArgs(const ArgList &Args,

Added: cfe/trunk/test/Driver/aarch64-outliner.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=331370&view=auto
==
--- cfe/trunk/test/Driver/aarch64-outliner.c (added)
+++ cfe/trunk/test/Driver/aarch64-outliner.c Wed May  2 09:42:51 2018
@@ -0,0 +1,4 @@
+// REQUIRES: aarch64-registered-target
+
+// RUN: %clang -target aarch64 -foutline -S %s -### 2>&1 | FileCheck %s
+// CHECK: "-mllvm" "-enable-machine-outliner"


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