[llvm-commits] CVS: llvm/lib/AsmParser/llvmAsmParser.y

Chris Lattner lattner at cs.uiuc.edu
Tue Jul 13 00:59:37 PDT 2004



Changes in directory llvm/lib/AsmParser:

llvmAsmParser.y updated: 1.173 -> 1.174

---
Log message:

A couple of substantial cleanup fixes:
  1. Split setValueName into two separate functions, one that is only used
     at function scope and doesn't have to deal with duplicates, and one
     that can be used either at global or function scope but that does deal
     with conflicts.  Conflicts were only in there because of the crappy old
     CFE and probably should be entirely eliminated.
  2. Insert BasicBlock's into the parent functions when they are created
     instead of when they are complete.  This effects name lookup (for the
     better), which will be exploited in the next patch.



---
Diffs of the changes:  (+72 -47)

Index: llvm/lib/AsmParser/llvmAsmParser.y
diff -u llvm/lib/AsmParser/llvmAsmParser.y:1.173 llvm/lib/AsmParser/llvmAsmParser.y:1.174
--- llvm/lib/AsmParser/llvmAsmParser.y:1.173	Tue Jul 13 01:58:07 2004
+++ llvm/lib/AsmParser/llvmAsmParser.y	Tue Jul 13 02:59:27 2004
@@ -479,36 +479,77 @@
 }
 
 
+static void setValueNameInternal(Value *V, const std::string &Name,
+                                 SymbolTable &ST) {
+  if (V->getType() == Type::VoidTy) 
+    ThrowException("Can't assign name '" + Name + "' to value with void type!");
+
+  // Set the name
+  V->setName(Name, &ST);
+
+  // If we're in function scope
+  if (inFunctionScope()) {
+    // Look up the symbol in the Function's local symboltable
+    Value *Existing = CurFun.LocalSymtab.lookup(V->getType(),Name);
+
+    // If it already exists, bail out.  We have to recheck this, because there
+    // may be entries inserted into the current basic block (which has not yet
+    // been inserted into the function) which collide.
+    if (Existing) {
+      ThrowException("Redefinition of value named '" + Name + "' in the '" +
+		   V->getType()->getDescription() + "' type plane!");
+    } else {
+      // otherwise, since it doesn't exist, insert it.
+      CurFun.LocalSymtab.insert(V);
+    }
+  }
+}
+
 // setValueName - Set the specified value to the name given.  The name may be
 // null potentially, in which case this is a noop.  The string passed in is
-// assumed to be a malloc'd string buffer, and is freed by this function.
+// assumed to be a malloc'd string buffer, and is free'd by this function.
+//
+static void setValueName(Value *V, char *NameStr) {
+  if (NameStr) {
+    std::string Name(NameStr);      // Copy string
+    free(NameStr);                  // Free old string
+    
+    assert(inFunctionScope() && "Must be in function scope!");
+    SymbolTable &ST = CurFun.CurrentFunction->getSymbolTable();
+    if (ST.lookup(V->getType(), Name))
+      ThrowException("Redefinition of value named '" + Name + "' in the '" +
+                     V->getType()->getDescription() + "' type plane!");
+    
+    setValueNameInternal(V, Name, ST);
+  }
+}
+
+// setValueNameMergingDuplicates - Set the specified value to the name given.
+// The name may be null potentially, in which case this is a noop.  The string
+// passed in is assumed to be a malloc'd string buffer, and is free'd by this
+// function.
 //
 // This function returns true if the value has already been defined, but is
 // allowed to be redefined in the specified context.  If the name is a new name
 // for the typeplane, false is returned.
 //
-static bool setValueName(Value *V, char *NameStr) {
+static bool setValueNameMergingDuplicates(Value *V, char *NameStr) {
   if (NameStr == 0) return false;
-  
+
   std::string Name(NameStr);      // Copy string
   free(NameStr);                  // Free old string
 
-  if (V->getType() == Type::VoidTy) 
-    ThrowException("Can't assign name '" + Name + 
-		   "' to a null valued instruction!");
-
+  // FIXME: If we eliminated the function constant pool (which we should), this
+  // would just unconditionally look at the module symtab.
   SymbolTable &ST = inFunctionScope() ? 
     CurFun.CurrentFunction->getSymbolTable() : 
     CurModule.CurrentModule->getSymbolTable();
 
   Value *Existing = ST.lookup(V->getType(), Name);
-
   if (Existing) {    // Inserting a name that is already defined???
-    // We are a simple redefinition of a value, check to see if it
-    // is defined the same as the old one...
-    if (const Constant *C = dyn_cast<Constant>(Existing)) {
-      if (C == V) return true;      // Constants are equal to themselves
-    } else if (GlobalVariable *EGV = dyn_cast<GlobalVariable>(Existing)) {
+    // We are a simple redefinition of a value, check to see if it is defined
+    // the same as the old one...
+    if (GlobalVariable *EGV = dyn_cast<GlobalVariable>(Existing)) {
       // We are allowed to redefine a global variable in two circumstances:
       // 1. If at least one of the globals is uninitialized or 
       // 2. If both initializers have the same value.
@@ -529,35 +570,20 @@
           return true;   // They are equivalent!
         }
       }
+    } else if (const Constant *C = dyn_cast<Constant>(Existing)) {
+      if (C == V) return true;      // Constants are equal to themselves
     }
 
     ThrowException("Redefinition of value named '" + Name + "' in the '" +
 		   V->getType()->getDescription() + "' type plane!");
   }
-
-  // Set the name
-  V->setName(Name, &ST);
-
-  // If we're in function scope
-  if (inFunctionScope()) {
-    // Look up the symbol in the function's local symboltable
-    Existing = CurFun.LocalSymtab.lookup(V->getType(),Name);
-
-    // If it already exists
-    if (Existing) {
-      // Bail
-      ThrowException("Redefinition of value named '" + Name + "' in the '" +
-		   V->getType()->getDescription() + "' type plane!");
-
-    // otherwise, since it doesn't exist
-    } else {
-      // Insert it.
-      CurFun.LocalSymtab.insert(V);
-    }
-  }
+  
+  setValueNameInternal(V, Name, ST);
   return false;
 }
 
+
+
 // setTypeName - Set the specified type to the name given.  The name may be
 // null potentially, in which case this is a noop.  The string passed in is
 // assumed to be a malloc'd string buffer, and is freed by this function.
@@ -1368,7 +1394,8 @@
 
 // ConstPool - Constants with optional names assigned to them.
 ConstPool : ConstPool OptAssign CONST ConstVal { 
-    if (!setValueName($4, $2))
+    // FIXME: THIS SHOULD REALLY BE ELIMINATED.  It is totally unneeded.
+    if (!setValueNameMergingDuplicates($4, $2))
       InsertValue($4);
   }
   | ConstPool OptAssign TYPE TypesV {  // Types can be defined in the const pool
@@ -1404,7 +1431,7 @@
       ThrowException("Global value initializer is not a constant!");
     
     GlobalVariable *GV = new GlobalVariable(Ty, $4, $3, Initializer);
-    if (!setValueName(GV, $2)) {   // If not redefining...
+    if (!setValueNameMergingDuplicates(GV, $2)) {   // If not redefining...
       CurModule.CurrentModule->getGlobalList().push_back(GV);
       int Slot = InsertValue(GV, CurModule.Values);
 
@@ -1420,7 +1447,7 @@
     const Type *Ty = *$5;
     // Global declarations appear in Constant Pool
     GlobalVariable *GV = new GlobalVariable(Ty,$4,GlobalValue::ExternalLinkage);
-    if (!setValueName(GV, $2)) {   // If not redefining...
+    if (!setValueNameMergingDuplicates(GV, $2)) {   // If not redefining...
       CurModule.CurrentModule->getGlobalList().push_back(GV);
       int Slot = InsertValue(GV, CurModule.Values);
 
@@ -1553,9 +1580,7 @@
          I != $4->end(); ++I, ++ArgIt) {
       delete I->first;                          // Delete the typeholder...
 
-      if (setValueName(ArgIt, I->second))       // Insert arg into symtab...
-        assert(0 && "No arg redef allowed!");
-      
+      setValueName(ArgIt, I->second);           // Insert arg into symtab...
       InsertValue(ArgIt);
     }
 
@@ -1635,10 +1660,10 @@
   };
 
 BasicBlockList : BasicBlockList BasicBlock {
-    ($$ = $1)->getBasicBlockList().push_back($2);
+    $$ = $1;
   }
   | FunctionHeader BasicBlock { // Do not allow functions with 0 basic blocks   
-    ($$ = $1)->getBasicBlockList().push_back($2);
+    $$ = $1;
   };
 
 
@@ -1646,7 +1671,7 @@
 // br, br/cc, switch, ret
 //
 BasicBlock : InstructionList OptAssign BBTerminatorInst  {
-    if (setValueName($3, $2)) { assert(0 && "No redefn allowed!"); }
+    setValueName($3, $2);
     InsertValue($3);
 
     $1->getInstList().push_back($3);
@@ -1654,11 +1679,11 @@
     $$ = $1;
   }
   | LABELSTR InstructionList OptAssign BBTerminatorInst  {
-    if (setValueName($4, $3)) { assert(0 && "No redefn allowed!"); }
+    setValueName($4, $3);
     InsertValue($4);
 
     $2->getInstList().push_back($4);
-    if (setValueName($2, $1)) { assert(0 && "No label redef allowed!"); }
+    setValueName($2, $1);
 
     InsertValue($2);
     $$ = $2;
@@ -1669,7 +1694,7 @@
     $$ = $1;
   }
   | /* empty */ {
-    $$ = CurBB = new BasicBlock();
+    $$ = CurBB = new BasicBlock("", CurFun.CurrentFunction);
   };
 
 BBTerminatorInst : RET ResolvedVal {              // Return with a result...
@@ -1782,7 +1807,7 @@
 
 Inst : OptAssign InstVal {
   // Is this definition named?? if so, assign the name...
-  if (setValueName($2, $1)) { assert(0 && "No redefin allowed!"); }
+  setValueName($2, $1);
   InsertValue($2);
   $$ = $2;
 };





More information about the llvm-commits mailing list