Skip to content

Updated ReplaceIRVar.cpp#18

Open
ayush-qubit wants to merge 9 commits intoreSHARMA:masterfrom
ayush-qubit:DebugInfo
Open

Updated ReplaceIRVar.cpp#18
ayush-qubit wants to merge 9 commits intoreSHARMA:masterfrom
ayush-qubit:DebugInfo

Conversation

@ayush-qubit
Copy link
Copy Markdown
Contributor

No description provided.

std::vector<Token *> GenericInstModel::extractPHINodeToken(llvm::Instruction *Inst){
std::vector<Token *> TokenVec;
TokenVec.push_back(this->getTokenWrapper()->getToken(Inst));
if(Inst->getOperand(0)->hasName()){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use hasName; I think you are using hasName to check if it a variable or a constant? if yes, prefer checking if it is ConstantInt, like we did in the StoreInst.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done this with the help of llvm::Constant

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that will be great. Though, hasName will also work fine but it will add an explicit dependency to run instnamer or something instnamer like to get it working.

else{
TokenVec.push_back(this->getTokenWrapper()->getToken(ValOp));
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be written as:

  if (!llvm::isa<llvm::ConstantInt>(ValOp)) return TokenVec;      

  if (llvm::isa<llvm::ConstantPointerNull>(ValOp))
    TokenVec.push_back(this->getTokenWrapper()->getToken("NULL",nullptr));
  else
    TokenVec.push_back(this->getTokenWrapper()->getToken(ValOp));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are trying to say

if (llvm::isa<llvm::ConstantInt>(ValOp)) return TokenVec;

if (llvm::isa<llvm::ConstantPointerNull>(ValOp))
TokenVec.push_back(this->getTokenWrapper()->getToken("NULL",nullptr));
else
TokenVec.push_back(this->getTokenWrapper()->getToken(ValOp));

And if it is then I have done this. Can I do this more general way like instead of using llvm::ConstantInt that can use only for integer constant we can use llvm::Constant which is more general way to say constant because constant can be float or double also.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please :)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that there can be other places where ConstantInt is used and it will fail with floats and doubles (thanks for pointing that out). Can you please create a github issue for that?

.gitignore Outdated
build/ No newline at end of file
build/
.vscode/
.github/ No newline at end of file
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.github has some action config and can not be put inside .gitignore.
Why do you need to add your gitignore to the PR?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will remove .vscode and .github but at the time of pushing code my build folder is also pushing to remove my build folder I put it in .gitignore.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you add files to commit? Do you do git add . or git commit -m, is yes, then try only adding the files that you want to be present in the commit.
For example, if you only want files inside lib and include to be in the commit then do git add lib/ include/

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed .gitignore

Copy link
Copy Markdown
Owner

@reSHARMA reSHARMA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR had changes for ReplaceIRVar, null token, and Phi node. Added some inline comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants