[PATCH] ARM IAS: support .personalityindex

Saleem Abdulrasool compnerd at compnerd.org
Sun Jan 19 12:15:35 PST 2014



================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8454
@@ -8432,1 +8453,3 @@
 bool ARMAsmParser::parseDirectivePersonality(SMLoc L) {
+  bool HasPersonality = UC.hasPersonality();
+
----------------
Logan Chien wrote:
> Renato Golin wrote:
> > style nit-pick: this local variable is unnecessary...
> It seems to be necessary because UC will be changed in the next line.  To make this clear, I would suggest either rename this variable to HasOtherPersonalityDirective or simply move the check in 8473 upward.
As Logan mentioned, this is used later on.  I've renamed it to HasExistingPersonality.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8791
@@ +8790,3 @@
+bool ARMAsmParser::parseDirectivePersonalityIndex(SMLoc L) {
+  bool HasPersonality = UC.hasPersonality();
+
----------------
Renato Golin wrote:
> unnecessary temp variable here, too
Same

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8832
@@ +8831,3 @@
+  }
+  if (CE->getValue() < 0 || CE->getValue() >= 3) {
+    Parser.eatToEndOfStatement();
----------------
Logan Chien wrote:
> Renato Golin wrote:
> > Logan Chien wrote:
> > > I would prefer to use:
> > > CE->getValue() < ARM::AEABI::NUM_PERSONALITY_INDEX
> > Good point. I agree.
> Oops.  I gave the incorrect code.  The correct one should be:
> 
>     if (CE->getValue() < 0 || CE->getValue() >= ARM::EHABI::NUM_PERSONALITY_INDEX) {
Changed to what Logan mentions in the last comment.

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:8834
@@ +8833,3 @@
+    Parser.eatToEndOfStatement();
+    Error(IndexLoc, "bad personality routine index");
+    return false;
----------------
Renato Golin wrote:
> a better error message would be to show it can't be > 3
Changed to:
    personality routine index should be in range [0-3]

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp:1146
@@ +1145,3 @@
+void ARMELFStreamer::emitPersonalityIndex(unsigned Index) {
+  assert(Index < NUM_PERSONALITY_INDEX && "invalid index");
+  PersonalityIndex = Index;
----------------
Logan Chien wrote:
> It seems that you will need ARM::EHABI:: here.
Done.

================
Comment at: lib/Target/ARM/MCTargetDesc/ARMUnwindOpAsm.cpp:199
@@ +198,3 @@
+    if (PersonalityIndex == ARM::EHABI::NUM_PERSONALITY_INDEX)
+      PersonalityIndex = Ops.size() <= 3 ? ARM::EHABI::AEABI_UNWIND_CPP_PR0
+                                         : ARM::EHABI::AEABI_UNWIND_CPP_PR1;
----------------
Renato Golin wrote:
> Logan Chien wrote:
> > Renato Golin wrote:
> > > Renato Golin wrote:
> > > > use of parenthesis here would be clearer.
> > > Here, too, the use of ARM::AEABI::NUM_PERSONALITY_INDEX.
> > The "3" in this line stands for the maximum unwind opcode that can be stored in compact mode pr0.  Currently, there is no named constant for this.
> oh, my bad.
Added parenthesis.


http://llvm-reviews.chandlerc.com/D2525



More information about the llvm-commits mailing list