Reid Spencer
2004-May-11 20:15 UTC
[LLVMdev] ExecutionEngine/Interpreter/ExternalFunctions.cpp
Hi, I'm working on bug 122, consolidating the interface to the SymbolTable class. In doing so, I found the function below which traverses the symbol table but apparently unnecessarily. Before I remove the traversal, I thought I better check with you guys. Posted this to the list because it looks like _everyone_ has edited this file :) In the code below, the IOB variable is the only thing in the loop that seems to get modified. However, it is never used in the subsequent code. Perhaps there's some subtle usage here I'm missing. I'd like to delete all the code from the start of the function to the end of the loop. Could someone who knows check this, please? Thanks, Reid. static FILE *getFILE(void *Ptr) { static Module *LastMod = 0; static PointerTy IOBBase = 0; static unsigned FILESize; if (LastMod != &TheInterpreter->getModule()) { // Module change or initialize? Module *M = LastMod = &TheInterpreter->getModule(); // Check to see if the currently loaded module contains an __iob symbol... GlobalVariable *IOB = 0; SymbolTable &ST = M->getSymbolTable(); for (SymbolTable::iterator I = ST.begin(), E = ST.end(); I != E; ++I) { SymbolTable::VarMap &M = I->second; for (SymbolTable::VarMap::iterator J = M.begin(), E = M.end(); J != E; ++J) if (J->first == "__iob") if ((IOB = dyn_cast<GlobalVariable>(J->second))) break; if (IOB) break; } } // Check to see if this is a reference to __iob... if (IOBBase) { unsigned FDNum = ((unsigned long)Ptr-IOBBase)/FILESize; if (FDNum == 0) return stdin; else if (FDNum == 1) return stdout; else if (FDNum == 2) return stderr; } return (FILE*)Ptr; } -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20040511/ac5d2e18/attachment.sig>
Reid Spencer
2004-May-11 20:29 UTC
[LLVMdev] ExecutionEngine/Interpreter/ExternalFunctions.cpp
I mis-stated what I think should be deleted. The block of code from "GlobalVariable *IOB = 0;" to the end of the loop should be delted because the only effect the loop has is on the IOB variable and that variable is never used after the loop. Reid. On Tue, 2004-05-11 at 18:14, Reid Spencer wrote:> Hi, > > I'm working on bug 122, consolidating the interface to the SymbolTable > class. In doing so, I found the function below which traverses the > symbol table but apparently unnecessarily. Before I remove the > traversal, I thought I better check with you guys. Posted this to the > list because it looks like _everyone_ has edited this file :) > > In the code below, the IOB variable is the only thing in the loop that > seems to get modified. However, it is never used in the subsequent code. > Perhaps there's some subtle usage here I'm missing. I'd like to delete > all the code from the start of the function to the end of the loop. > > Could someone who knows check this, please? > > Thanks, > > Reid. > > > static FILE *getFILE(void *Ptr) { > static Module *LastMod = 0; > static PointerTy IOBBase = 0; > static unsigned FILESize; > > if (LastMod != &TheInterpreter->getModule()) { // Module change or initialize? > Module *M = LastMod = &TheInterpreter->getModule(); > > // Check to see if the currently loaded module contains an __iob symbol... > GlobalVariable *IOB = 0; > SymbolTable &ST = M->getSymbolTable(); > for (SymbolTable::iterator I = ST.begin(), E = ST.end(); I != E; ++I) { > SymbolTable::VarMap &M = I->second; > for (SymbolTable::VarMap::iterator J = M.begin(), E = M.end(); > J != E; ++J) > if (J->first == "__iob") > if ((IOB = dyn_cast<GlobalVariable>(J->second))) > break; > if (IOB) break; > } > } > > // Check to see if this is a reference to __iob... > if (IOBBase) { > unsigned FDNum = ((unsigned long)Ptr-IOBBase)/FILESize; > if (FDNum == 0) > return stdin; > else if (FDNum == 1) > return stdout; > else if (FDNum == 2) > return stderr; > } > > return (FILE*)Ptr; > }_______________________ Reid Spencer President & CTO eXtensible Systems, Inc. rspencer at x10sys.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20040511/35056102/attachment.sig>
Reid Spencer
2004-May-11 20:34 UTC
[LLVMdev] ExecutionEngine/Interpreter/ExternalFunctions.cpp
And, one more weird thing in this function. The FILESize static variable is never initialized so its likely initial value is 0 due to zero fill on many MMUs. The value is never written and used as a divisor. Why hasn't this function caused an arithmetic violation? Because the IOBBase point, also a static variable is initialized to zero and never modified and used in a conditional that thwarts the second if statement. This function amounts to a hugely expensive cast to File* on its argument! What was the _intent_ of all this? Reid. On Tue, 2004-05-11 at 18:28, Reid Spencer wrote:> I mis-stated what I think should be deleted. > > The block of code from "GlobalVariable *IOB = 0;" to the end of the loop > should be delted because the only effect the loop has is on the IOB > variable and that variable is never used after the loop. > > Reid. > > On Tue, 2004-05-11 at 18:14, Reid Spencer wrote: > > Hi, > > > > I'm working on bug 122, consolidating the interface to the SymbolTable > > class. In doing so, I found the function below which traverses the > > symbol table but apparently unnecessarily. Before I remove the > > traversal, I thought I better check with you guys. Posted this to the > > list because it looks like _everyone_ has edited this file :) > > > > In the code below, the IOB variable is the only thing in the loop that > > seems to get modified. However, it is never used in the subsequent code. > > Perhaps there's some subtle usage here I'm missing. I'd like to delete > > all the code from the start of the function to the end of the loop. > > > > Could someone who knows check this, please? > > > > Thanks, > > > > Reid. > > > > > > static FILE *getFILE(void *Ptr) { > > static Module *LastMod = 0; > > static PointerTy IOBBase = 0; > > static unsigned FILESize; > > > > if (LastMod != &TheInterpreter->getModule()) { // Module change or initialize? > > Module *M = LastMod = &TheInterpreter->getModule(); > > > > // Check to see if the currently loaded module contains an __iob symbol... > > GlobalVariable *IOB = 0; > > SymbolTable &ST = M->getSymbolTable(); > > for (SymbolTable::iterator I = ST.begin(), E = ST.end(); I != E; ++I) { > > SymbolTable::VarMap &M = I->second; > > for (SymbolTable::VarMap::iterator J = M.begin(), E = M.end(); > > J != E; ++J) > > if (J->first == "__iob") > > if ((IOB = dyn_cast<GlobalVariable>(J->second))) > > break; > > if (IOB) break; > > } > > } > > > > // Check to see if this is a reference to __iob... > > if (IOBBase) { > > unsigned FDNum = ((unsigned long)Ptr-IOBBase)/FILESize; > > if (FDNum == 0) > > return stdin; > > else if (FDNum == 1) > > return stdout; > > else if (FDNum == 2) > > return stderr; > > } > > > > return (FILE*)Ptr; > > } > > > _______________________ > Reid Spencer > President & CTO > eXtensible Systems, Inc.rspencer at x10sys.com _______________________ Reid Spencer President & CTO eXtensible Systems, Inc. rspencer at x10sys.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 189 bytes Desc: This is a digitally signed message part URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20040511/462939f5/attachment.sig>
Chris Lattner
2004-May-11 20:59 UTC
[LLVMdev] ExecutionEngine/Interpreter/ExternalFunctions.cpp
On Tue, 11 May 2004, Reid Spencer wrote:> I mis-stated what I think should be deleted. > > The block of code from "GlobalVariable *IOB = 0;" to the end of the loop > should be delted because the only effect the loop has is on the IOB > variable and that variable is never used after the loop.Yeah, that code can be deleted. The "__iob" emulation code has been removed and that must have been missed. Thanks Reid! -Chris> On Tue, 2004-05-11 at 18:14, Reid Spencer wrote: > > Hi, > > > > I'm working on bug 122, consolidating the interface to the SymbolTable > > class. In doing so, I found the function below which traverses the > > symbol table but apparently unnecessarily. Before I remove the > > traversal, I thought I better check with you guys. Posted this to the > > list because it looks like _everyone_ has edited this file :) > > > > In the code below, the IOB variable is the only thing in the loop that > > seems to get modified. However, it is never used in the subsequent code. > > Perhaps there's some subtle usage here I'm missing. I'd like to delete > > all the code from the start of the function to the end of the loop. > > > > Could someone who knows check this, please? > > > > Thanks, > > > > Reid. > > > > > > static FILE *getFILE(void *Ptr) { > > static Module *LastMod = 0; > > static PointerTy IOBBase = 0; > > static unsigned FILESize; > > > > if (LastMod != &TheInterpreter->getModule()) { // Module change or initialize? > > Module *M = LastMod = &TheInterpreter->getModule(); > > > > // Check to see if the currently loaded module contains an __iob symbol... > > GlobalVariable *IOB = 0; > > SymbolTable &ST = M->getSymbolTable(); > > for (SymbolTable::iterator I = ST.begin(), E = ST.end(); I != E; ++I) { > > SymbolTable::VarMap &M = I->second; > > for (SymbolTable::VarMap::iterator J = M.begin(), E = M.end(); > > J != E; ++J) > > if (J->first == "__iob") > > if ((IOB = dyn_cast<GlobalVariable>(J->second))) > > break; > > if (IOB) break; > > } > > } > > > > // Check to see if this is a reference to __iob... > > if (IOBBase) { > > unsigned FDNum = ((unsigned long)Ptr-IOBBase)/FILESize; > > if (FDNum == 0) > > return stdin; > > else if (FDNum == 1) > > return stdout; > > else if (FDNum == 2) > > return stderr; > > } > > > > return (FILE*)Ptr; > > } > > > _______________________ > Reid Spencer > President & CTO > eXtensible Systems, Inc. > rspencer at x10sys.com >-Chris -- http://llvm.cs.uiuc.edu/ http://www.nondot.org/~sabre/Projects/