This revision was automatically updated to reflect the committed changes.
Closed by commit rL276958: [AArch64] Using AArch64TargetParser in Clang.
(authored by zijiaoma).
Changed prior to commit:
https://reviews.llvm.org/D21277?vs=60658&id=65878#toc
Repository:
rL LLVM
https://reviews.llvm.
jojo added a comment.
I'm sorry that I should have submitted this and its fix in one review.
Now the old fix http://reviews.llvm.org/D21276 is abandoned,as a latest fix is
included in http://reviews.llvm.org/D21785.
I will commit http://reviews.llvm.org/D21277 and
http://reviews.llvm.org/D21785,
rengolin reopened this revision.
rengolin added a comment.
This revision is now accepted and ready to land.
Reopening, as this is the right patch.
This patch is approved, waiting on http://reviews.llvm.org/D21785, which fixes
the issue that made us revert http://reviews.llvm.org/D20088 in the fi
rengolin added a comment.
Sorry, this was committed and reverted (bugs, being fixed by
http://reviews.llvm.org/D21785). We should start a new one when that's in.
Ignore this one. :)
Repository:
rL LLVM
http://reviews.llvm.org/D21277
___
cfe-com
mehdi_amini added a comment.
Why was it closed? Was it committed? It'd be nice to have a comment with the
closing action.
Repository:
rL LLVM
http://reviews.llvm.org/D21277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llv
echristo accepted this revision.
echristo added a comment.
This revision is now accepted and ready to land.
Fine with me.
-eric
Comment at: lib/Basic/Targets.cpp:5709
@@ -5716,3 +5708,3 @@
void getTargetDefines(const LangOptions &Opts,
MacroBuilder
rengolin added a comment.
In http://reviews.llvm.org/D21277#459703, @jojo wrote:
> Do you mean updating the diff to let it include the change of
> http://reviews.llvm.org/D21276,or commiting these two reviews as one commit?
Committing them as one, after they're both approved.
Repository:
r
jojo added a comment.
> I recommend you squash this patch with D21276 before commit.
Hi, Renato,
Do you mean updating the diff to let it include the change of
http://reviews.llvm.org/D21276,or commiting these two reviews as one commit?
Repository:
rL LLVM
http://reviews.llvm.org/D21277
rengolin added a comment.
Hi Jojo,
This looks good to me, and I recommend you squash this patch with
http://reviews.llvm.org/D21276 before commit. But I'll let @echristo and
@compnerd have the final say.
cheers,
--renato
Repository:
rL LLVM
http://reviews.llvm.org/D21277
__
rengolin added inline comments.
Comment at: lib/Basic/Targets.cpp:5709
@@ -5716,3 +5708,3 @@
void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override {
We'll have to re-work these parts anyway. When I was l
jojo updated this revision to Diff 60658.
jojo added a comment.
Herald added a subscriber: mehdi_amini.
Update according to inline comments.
Repository:
rL LLVM
http://reviews.llvm.org/D21277
Files:
lib/Basic/Targets.cpp
lib/Driver/Tools.cpp
Index: lib/Driver/Tools.cpp
=
jojo added inline comments.
Comment at: lib/Basic/Targets.cpp:5709
@@ -5715,1 +5708,3 @@
+
+return false;
}
echristo wrote:
> compnerd wrote:
> > Please collapse this:
> >
> > return Name == "generic" || llvm::AArch64::parseCPUArch(Name) !=
> > llvm::
echristo added inline comments.
Comment at: lib/Basic/Targets.cpp:5709
@@ -5715,1 +5708,3 @@
+
+return false;
}
compnerd wrote:
> Please collapse this:
>
> return Name == "generic" || llvm::AArch64::parseCPUArch(Name) !=
> llvm::ARM::AK_INVALID;
... w
compnerd added a subscriber: compnerd.
Comment at: lib/Basic/Targets.cpp:5709
@@ -5715,1 +5708,3 @@
+
+return false;
}
Please collapse this:
return Name == "generic" || llvm::AArch64::parseCPUArch(Name) !=
llvm::ARM::AK_INVALID;
Comm
jojo created this revision.
jojo added reviewers: rengolin, jmolloy, bsmith, rsmith, labrinea.
jojo added a subscriber: cfe-commits.
jojo set the repository for this revision to rL LLVM.
jojo changed the visibility of this Differential Revision from "Public (No
Login Required)" to "All Users".
Her
15 matches
Mail list logo
