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
14 matches
Mail list logo
