[llvm] r199886 - Prevent repetitive warnings for unrecognized processors and features
Sean Silva
silvas at purdue.edu
Thu Jan 23 11:35:29 PST 2014
+// This needs to be shared between the instantiations of Find<>
+typedef std::pair<std::string,std::string> KeyWithType;
+static SmallSet<KeyWithType,10> reportedAsUnrecognized;
+
This is introducing a global into the libraries, which is verboten. Please
revert and find a design that doesn't require the global.
This should have been caught in pre-commit review.
+
+ /// Find KV in array using binary search.
+ /// T should be either SubtargetFeatureKV or SubtargetInfoKV
+ template<typename T>
+ static const T *Find(StringRef Key, const T *Array, size_t Length,
+ const char* KeyType);
This should be `find` according to the naming convention. Also, You should
use ArrayRef instead of a raw pointer + length. Maybe a more descriptive
name than `find` would be useful as well.
-- Sean Silva
On Thu, Jan 23, 2014 at 6:31 AM, Artyom Skrobov <Artyom.Skrobov at arm.com>wrote:
> Author: askrobov
> Date: Thu Jan 23 05:31:38 2014
> New Revision: 199886
>
> URL: http://llvm.org/viewvc/llvm-project?rev=199886&view=rev
> Log:
> Prevent repetitive warnings for unrecognized processors and features
>
> Added:
> llvm/trunk/test/MC/ARM/unrecognized.ll
> Modified:
> llvm/trunk/include/llvm/MC/SubtargetFeature.h
> llvm/trunk/lib/MC/MCSubtargetInfo.cpp
> llvm/trunk/lib/MC/SubtargetFeature.cpp
>
> Modified: llvm/trunk/include/llvm/MC/SubtargetFeature.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/SubtargetFeature.h?rev=199886&r1=199885&r2=199886&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/MC/SubtargetFeature.h (original)
> +++ llvm/trunk/include/llvm/MC/SubtargetFeature.h Thu Jan 23 05:31:38 2014
> @@ -101,6 +101,12 @@ public:
>
> /// Adds the default features for the specified target triple.
> void getDefaultSubtargetFeatures(const Triple& Triple);
> +
> + /// Find KV in array using binary search.
> + /// T should be either SubtargetFeatureKV or SubtargetInfoKV
> + template<typename T>
> + static const T *Find(StringRef Key, const T *Array, size_t Length,
> + const char* KeyType);
> };
>
> } // End namespace llvm
>
> Modified: llvm/trunk/lib/MC/MCSubtargetInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCSubtargetInfo.cpp?rev=199886&r1=199885&r2=199886&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/MCSubtargetInfo.cpp (original)
> +++ llvm/trunk/lib/MC/MCSubtargetInfo.cpp Thu Jan 23 05:31:38 2014
> @@ -96,14 +96,11 @@ MCSubtargetInfo::getSchedModelForCPU(Str
> #endif
>
> // Find entry
> - const SubtargetInfoKV *Found =
> - std::lower_bound(ProcSchedModels, ProcSchedModels+NumProcs, CPU);
> - if (Found == ProcSchedModels+NumProcs || StringRef(Found->Key) != CPU) {
> - errs() << "'" << CPU
> - << "' is not a recognized processor for this target"
> - << " (ignoring processor)\n";
> + const SubtargetInfoKV *Found = SubtargetFeatures::Find(CPU,
> ProcSchedModels,
> + NumProcs,
> "processor");
> + if (!Found)
> return &MCSchedModel::DefaultSchedModel;
> - }
> +
> assert(Found->Value && "Missing processor SchedModel value");
> return (const MCSchedModel *)Found->Value;
> }
>
> Modified: llvm/trunk/lib/MC/SubtargetFeature.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/SubtargetFeature.cpp?rev=199886&r1=199885&r2=199886&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/MC/SubtargetFeature.cpp (original)
> +++ llvm/trunk/lib/MC/SubtargetFeature.cpp Thu Jan 23 05:31:38 2014
> @@ -11,9 +11,11 @@
> //
>
> //===----------------------------------------------------------------------===//
>
> +#include "llvm/ADT/SmallSet.h"
> #include "llvm/MC/SubtargetFeature.h"
> #include "llvm/Support/Debug.h"
> #include "llvm/Support/Format.h"
> +#include "llvm/Support/SourceMgr.h"
> #include "llvm/Support/raw_ostream.h"
> #include <algorithm>
> #include <cassert>
> @@ -118,19 +120,42 @@ void SubtargetFeatures::AddFeature(const
> }
> }
>
> +// This needs to be shared between the instantiations of Find<>
> +typedef std::pair<std::string,std::string> KeyWithType;
> +static SmallSet<KeyWithType,10> reportedAsUnrecognized;
> +
> /// Find KV in array using binary search.
> -static const SubtargetFeatureKV *Find(StringRef S, const
> SubtargetFeatureKV *A,
> - size_t L) {
> +template<typename T>
> +const T *SubtargetFeatures::Find(StringRef Key, const T *Array, size_t
> Length,
> + const char* KeyType) {
> // Determine the end of the array
> - const SubtargetFeatureKV *Hi = A + L;
> + const T *Hi = Array + Length;
> // Binary search the array
> - const SubtargetFeatureKV *F = std::lower_bound(A, Hi, S);
> + const T *F = std::lower_bound(Array, Hi, Key);
> // If not found then return NULL
> - if (F == Hi || StringRef(F->Key) != S) return NULL;
> + if (F == Hi || StringRef(F->Key) != Key) {
> + // If not yet reported, report now
> + KeyWithType current(KeyType, Key);
> + if(!reportedAsUnrecognized.count(current)) {
> + SmallString<1024> storage;
> + StringRef message = ("'" + Key +
> + "' is not a recognized " + KeyType +
> + " for this target (ignoring " + KeyType +
> + ")").toStringRef(storage);
> + SMDiagnostic(StringRef(), SourceMgr::DK_Warning, message)
> + .print(0, errs());
> + reportedAsUnrecognized.insert(current);
> + }
> + return NULL;
> + }
> // Return the found array item
> return F;
> }
>
> +// Instantiate with <SubtargetInfoKV> for use in MCSubtargetInfo
> +template const SubtargetInfoKV *SubtargetFeatures::Find<SubtargetInfoKV>
> + (StringRef Key, const SubtargetInfoKV *Array, size_t Length, const
> char* KeyType);
> +
> /// getLongestEntryLength - Return the length of the longest entry in the
> table.
> ///
> static size_t getLongestEntryLength(const SubtargetFeatureKV *Table,
> @@ -228,7 +253,7 @@ SubtargetFeatures::ToggleFeature(uint64_
> size_t FeatureTableSize) {
> // Find feature in table.
> const SubtargetFeatureKV *FeatureEntry =
> - Find(StripFlag(Feature), FeatureTable, FeatureTableSize);
> + Find(StripFlag(Feature), FeatureTable, FeatureTableSize, "feature");
> // If there is a match
> if (FeatureEntry) {
> if ((Bits & FeatureEntry->Value) == FeatureEntry->Value) {
> @@ -242,10 +267,6 @@ SubtargetFeatures::ToggleFeature(uint64_
> // For each feature that this implies, set it.
> SetImpliedBits(Bits, FeatureEntry, FeatureTable, FeatureTableSize);
> }
> - } else {
> - errs() << "'" << Feature
> - << "' is not a recognized feature for this target"
> - << " (ignoring feature)\n";
> }
>
> return Bits;
> @@ -280,7 +301,8 @@ uint64_t SubtargetFeatures::getFeatureBi
>
> // Find CPU entry if CPU name is specified.
> if (!CPU.empty()) {
> - const SubtargetFeatureKV *CPUEntry = Find(CPU, CPUTable,
> CPUTableSize);
> + const SubtargetFeatureKV *CPUEntry = Find(CPU, CPUTable, CPUTableSize,
> + "processor");
> // If there is a match
> if (CPUEntry) {
> // Set base feature bits
> @@ -292,10 +314,6 @@ uint64_t SubtargetFeatures::getFeatureBi
> if (CPUEntry->Value & FE.Value)
> SetImpliedBits(Bits, &FE, FeatureTable, FeatureTableSize);
> }
> - } else {
> - errs() << "'" << CPU
> - << "' is not a recognized processor for this target"
> - << " (ignoring processor)\n";
> }
> }
>
> @@ -309,7 +327,7 @@ uint64_t SubtargetFeatures::getFeatureBi
>
> // Find feature in table.
> const SubtargetFeatureKV *FeatureEntry =
> - Find(StripFlag(Feature), FeatureTable,
> FeatureTableSize);
> + Find(StripFlag(Feature), FeatureTable, FeatureTableSize,
> "feature");
> // If there is a match
> if (FeatureEntry) {
> // Enable/disable feature in bits
> @@ -324,10 +342,6 @@ uint64_t SubtargetFeatures::getFeatureBi
> // For each feature that implies this, clear it.
> ClearImpliedBits(Bits, FeatureEntry, FeatureTable,
> FeatureTableSize);
> }
> - } else {
> - errs() << "'" << Feature
> - << "' is not a recognized feature for this target"
> - << " (ignoring feature)\n";
> }
> }
>
>
> Added: llvm/trunk/test/MC/ARM/unrecognized.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/ARM/unrecognized.ll?rev=199886&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/MC/ARM/unrecognized.ll (added)
> +++ llvm/trunk/test/MC/ARM/unrecognized.ll Thu Jan 23 05:31:38 2014
> @@ -0,0 +1,15 @@
> +; RUN: llc -mcpu=invalid -o /dev/null %s 2>&1 | FileCheck %s
> --check-prefix=CPU
> +; CPU: 'invalid' is not a recognized processor for this target
> (ignoring processor)
> +; CPU-NOT: 'invalid' is not a recognized processor for this target
> (ignoring processor)
> +
> +; RUN: llc -mattr=+foo,+bar -o /dev/null %s 2>&1 | FileCheck %s
> --check-prefix=FEATURE
> +; FEATURE: 'foo' is not a recognized feature for this target
> (ignoring feature)
> +; FEATURE-NEXT: 'bar' is not a recognized feature for this target
> (ignoring feature)
> +; FEATURE-NOT: 'foo' is not a recognized feature for this target
> (ignoring feature)
> +; FEATURE-NOT: 'bar' is not a recognized feature for this target
> (ignoring feature)
> +
> +define void @foo() {
> +entry:
> + ret void
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140123/7fa280fd/attachment.html>
More information about the llvm-commits
mailing list