Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cacheloads flag related fixes so insertsort_alt works with cacheloads=true #18

Closed
wants to merge 17 commits into from

Conversation

timkaler
Copy link
Contributor

No description provided.

@timkaler timkaler requested a review from wsmoses October 22, 2019 20:07
@wsmoses
Copy link
Member

wsmoses commented Oct 22, 2019

Can you rebase off the current plugin branch so I can see what the differences are (this seems to include prior commits which makes reading it harder$

@timkaler
Copy link
Contributor Author

Can you rebase off the current plugin branch so I can see what the differences are (this seems to include prior commits which makes reading it harder$

The actual diff is pretty small. You should be able to just quickly scan through all changes to sanity check the diff. The extraneous changes are from pulling plugin so to allow for an automatic merge.

Also, in case there's confusion: you originally created this "morefix" branch. For some reason there are 2 branches (morefix and simplefix) that both have the commit which added the cachereads flag.

wsmoses
wsmoses previously approved these changes Oct 22, 2019
inst = cast<Instruction>(gutils->addMalloc(BuilderZ, inst));
llvm::errs() << "before cast inst :" << *inst << "\n";
llvm::errs() << "Cast successful " << "\n";
//llvm::errs() << "Forcibly loading cached reads after addmalloc " << *op << "\n";
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove these prints

@wsmoses
Copy link
Member

wsmoses commented Oct 22, 2019

Tentatively approved but please remove prints. Also please do rebase and merge rather than squash and merge so the previous fixes in that branch are separate. Otherwise LGTM

@wsmoses
Copy link
Member

wsmoses commented Oct 23, 2019

Closing this, see #19

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.

None yet

2 participants