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
[te] Fix casting of unsigned char, and abs(int) #44157
Changes from all commits
8e2e2e2
c2744f3
59273cb
0827798
23bbc45
cf87af0
0dd0f65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,7 +110,7 @@ std::string cudaDtypeCppString(const Dtype& dtype) { | |
case ScalarType::Short: | ||
return "short"; | ||
case ScalarType::Long: | ||
return "long"; | ||
return "long long"; | ||
default: | ||
return dtype.ToCppString(); | ||
} | ||
|
@@ -198,7 +198,7 @@ void CudaPrinter::visit(const Cast* v) { | |
return; | ||
} | ||
|
||
os() << cudaDtypeCppString(v->dtype()); | ||
os() << "(" << cudaDtypeCppString(v->dtype()) << ")"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ouch, how did it work? :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know when it became a thing but apparently you can cast by saying |
||
os() << "("; | ||
v->src_value()->accept(this); | ||
os() << ")"; | ||
|
@@ -221,6 +221,9 @@ void CudaPrinter::visit(const Intrinsics* v) { | |
if (returnType == ScalarType::Half || returnType == ScalarType::Float) { | ||
func_name = func_name + "f"; | ||
} | ||
if (v->op_type() == IntrinsicsOp::kFabs && is_integral(returnType)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does nvrtc not handle std::abs or ::abs? All this exp/expf is so last decade (and is not used in eager part of the codebase at all). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it does, actually. I don't know why we went the |
||
func_name = "abs"; | ||
} | ||
|
||
os() << func_name << "("; | ||
for (int i = 0; i < v->nparams(); i++) { | ||
|
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.
Nit: maybe add a comment whether we want these to be supported at all?
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.
Oh, yeah, float16 should be coming in soon-ish. The commented lines are a transient state for this stack :-)