-
Notifications
You must be signed in to change notification settings - Fork 93
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
Calculate adjoint of cblas_dgemm #308
Conversation
b22c958
to
7b29c04
Compare
@reikdas bumping a rebase of this if possible |
7b29c04
to
5013fd8
Compare
7755f94
to
4102362
Compare
4102362
to
1c3b19d
Compare
1c3b19d
to
b1c7d8b
Compare
@wsmoses The CI failures should be unrelated. |
Yup, those CI failures are needing to get https://reviews.llvm.org/D108221 landed. |
uncacheable_args.find(xfuncarg)->second; | ||
bool ycache = !gutils->isConstantValue(call.getArgOperand(1)) && | ||
uncacheable_args.find(yfuncarg)->second; | ||
if ((Mode == DerivativeMode::ReverseModeCombined || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add asserts that the forward modes are unhandled
if (!Bcache) | ||
structarg2 = lookup(gutils->getNewFromOriginal(call.getArgOperand(9)), | ||
Builder2); | ||
if (cast<ConstantInt>(call.getArgOperand(0))->getValue() == 102) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you compare with a character rather than a raw number? Also usually don't both upper and lowercase work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first three arguments are enums. I am not sure about chars that we can pass as arguments instead. Do you have an example?
Do you instead mean passing in the enum names like CblasRowMajor
, CblasNoTrans
, etc instead of the number?
CallInst *safunccall, *sbfunccall; | ||
if (aactive) { | ||
if (cast<ConstantInt>(call.getArgOperand(2))->getValue() == 112 || | ||
cast<ConstantInt>(call.getArgOperand(2))->getValue() == 113) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise character for the rest here.
sbfunccall = Builder2.CreateCall(dfunc, sbfuncargs); | ||
} | ||
auto scfunc = gutils->oldFunc->getParent()->getOrInsertFunction( | ||
scfuncname, Builder2.getVoidTy(), Builder2.getInt32Ty(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we always sure this is an int32? Can we instead use whatever the type of the constantint is (if, for some reason, it is say an int64, etc).
entry: | ||
%A = alloca [8 x double], align 16 | ||
%B = alloca [6 x double], align 16 | ||
%C = alloca [12 x double], align 16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the main except for the call to autodiff (and perhaps just have arguments forwarded to the autodiff call)
;CHECK-NEXT: %4 = bitcast double* %2 to i8* | ||
;CHECK-NEXT: tail call void @free(i8* %4) | ||
;CHECK-NEXT: call void @cblas_dscal(i32 12, double %beta, double* %"Cfoo'", i32 1) | ||
;CHECK-NEXT: ret { double, double } zeroinitializer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this zero? Shouldn't we also consider the case where alpha and beta are active?
No description provided.