This revision was automatically updated to reflect the committed changes.
Closed by commit rL304994: Frontend support for Nios2 target. (authored by
n.bozhenov).
Changed prior to commit:
https://reviews.llvm.org/D33356?vs=100674=101941#toc
Repository:
rL LLVM
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: lib/Basic/Targets.cpp:7678
+for (const char *feature : allFeatures) {
+Features[feature] = isFeatureSupportedByCPU(feature, CPU);
+
belickim updated this revision to Diff 100674.
belickim added a comment.
Thanks Akira for your comments, I made all changes that you suggested.
https://reviews.llvm.org/D33356
Files:
include/clang/Basic/BuiltinsNios2.def
include/clang/Basic/TargetBuiltins.h
lib/Basic/Targets.cpp
belickim added inline comments.
Comment at: include/clang/Basic/TargetBuiltins.h:154
+ /// \brief Nios2 builtins
+ namespace Nios2 {
+ enum {
ahatanak wrote:
> No need to indent enum inside namespace.
>
>
ahatanak added a comment.
A couple of coding style nits. FYI, LLVM coding standard is documented here:
http://llvm.org/docs/CodingStandards.html
Comment at: include/clang/Basic/TargetBuiltins.h:154
+ /// \brief Nios2 builtins
+ namespace Nios2 {
+ enum {
belickim updated this revision to Diff 100668.
belickim added a comment.
I added test for driver to check if target and CPU flags are recognized, as
Craig suggested.
https://reviews.llvm.org/D33356
Files:
include/clang/Basic/BuiltinsNios2.def
include/clang/Basic/TargetBuiltins.h
craig.topper added a comment.
Is there enough functional here that there should be tests for? i.e. make sure
march/mcpu switches are recognized, that the target is recognized, etc.
https://reviews.llvm.org/D33356
___
cfe-commits mailing list
joerg added inline comments.
Comment at: lib/Basic/Targets.cpp:7650
+DefineStd(Builder, "nios2", Opts);
+DefineStd(Builder, "NIOS2", Opts);
+
belickim wrote:
> joerg wrote:
> > belickim wrote:
> > > joerg wrote:
> > > > DefineStd tends to give a lot of
belickim added inline comments.
Comment at: lib/Basic/Targets.cpp:7650
+DefineStd(Builder, "nios2", Opts);
+DefineStd(Builder, "NIOS2", Opts);
+
joerg wrote:
> belickim wrote:
> > joerg wrote:
> > > DefineStd tends to give a lot of namespace pollution,
joerg added inline comments.
Comment at: lib/Basic/Targets.cpp:7650
+DefineStd(Builder, "nios2", Opts);
+DefineStd(Builder, "NIOS2", Opts);
+
belickim wrote:
> joerg wrote:
> > DefineStd tends to give a lot of namespace pollution, is that intentional
>
belickim added a comment.
Additionally this information may be useful in context of this patch: here is
specification of Nios2 builtins as implemented in GCC:
https://gcc.gnu.org/onlinedocs/gcc/Altera-Nios-II-Built-in-Functions.html
https://reviews.llvm.org/D33356
belickim added inline comments.
Comment at: lib/Basic/Targets.cpp:7614
+ resetDataLayout(("e-" + Layout).str());
+}
+ }
joerg wrote:
> Can you just give the full string and avoid the runtime allocations?
Yes, I will fix this.
Comment
joerg added inline comments.
Comment at: lib/Basic/Targets.cpp:7614
+ resetDataLayout(("e-" + Layout).str());
+}
+ }
Can you just give the full string and avoid the runtime allocations?
Comment at: lib/Basic/Targets.cpp:7650
+
belickim created this revision.
Hello,
This patch is part of our effort to implement Nios2 architecture support in
LLVM and clang (our plans were announced in mailing list thread: "[llvm-dev]
[RFC] Nios II backend" form April 12th).
This patch depends on changes introduced by patch D32669
14 matches
Mail list logo
