-
Notifications
You must be signed in to change notification settings - Fork 187
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
WIP: Emit header files for cpp, cuda and metal targets #3938
base: master
Are you sure you want to change the base?
Changes from all commits
84891ed
c87cfef
269219e
8dee1a9
473a648
f118939
3e409f4
f1a1664
809528e
b44a14f
994e480
cd42390
88e69da
59d3300
de740ec
381a9d0
20859f7
7e0f54f
2775c63
bb3b5bd
e0f3a10
0fd14ab
6f3ba04
91fd1ec
e1c8ad2
c360a96
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 |
---|---|---|
|
@@ -82,16 +82,19 @@ struct CLikeSourceEmitter::ComputeEmitActionsContext | |
return SourceLanguage::C; | ||
} | ||
case CodeGenTarget::CPPSource: | ||
case CodeGenTarget::CPPHeader: | ||
case CodeGenTarget::HostCPPSource: | ||
case CodeGenTarget::PyTorchCppBinding: | ||
{ | ||
return SourceLanguage::CPP; | ||
} | ||
case CodeGenTarget::CUDASource: | ||
case CodeGenTarget::CUDAHeader: | ||
{ | ||
return SourceLanguage::CUDA; | ||
} | ||
case CodeGenTarget::Metal: | ||
case CodeGenTarget::MetalHeader: | ||
{ | ||
return SourceLanguage::Metal; | ||
} | ||
|
@@ -1150,7 +1153,20 @@ String CLikeSourceEmitter::getName(IRInst* inst) | |
String name; | ||
if(!m_mapInstToName.tryGetValue(inst, name)) | ||
{ | ||
name = generateName(inst); | ||
// unmangle names, when emitting header | ||
if (shouldEmitOnlyHeader()) | ||
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 we should bottleneck name generating logic through the same branch regardless of |
||
{ | ||
if (auto nameHintDecor = inst->findDecoration<IRNameHintDecoration>()) | ||
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. This feels hacky. We should use the same name emit logic for header and cpp files. For things that needs to be emitted without name mangling, they should be marked as extern_cpp. |
||
{ | ||
StringBuilder sb; | ||
appendScrubbedName(nameHintDecor->getName(), sb); | ||
name = sb.produceString(); | ||
} | ||
} | ||
else | ||
{ | ||
name = generateName(inst); | ||
} | ||
m_mapInstToName.add(inst, name); | ||
} | ||
return name; | ||
|
@@ -3476,7 +3492,7 @@ void CLikeSourceEmitter::emitSimpleFuncImpl(IRFunc* func) | |
emitSemantics(func); | ||
|
||
// TODO: encode declaration vs. definition | ||
if(isDefinition(func)) | ||
if(!shouldEmitOnlyHeader() && isDefinition(func)) | ||
{ | ||
m_writer->emit("\n{\n"); | ||
m_writer->indent(); | ||
|
@@ -3622,6 +3638,11 @@ bool shouldWrapInExternCBlock(IRFunc* func) | |
|
||
void CLikeSourceEmitter::emitFunc(IRFunc* func) | ||
{ | ||
if (shouldEmitOnlyHeader() && !func->findDecoration<IRExternCppDecoration>()) | ||
{ | ||
return; | ||
} | ||
|
||
// Target-intrinsic functions should never be emitted | ||
// even if they happen to have a body. | ||
// | ||
|
@@ -3669,6 +3690,11 @@ void CLikeSourceEmitter::emitFuncDecorationsImpl(IRFunc* func) | |
|
||
void CLikeSourceEmitter::emitStruct(IRStructType* structType) | ||
{ | ||
if (shouldEmitOnlyHeader() && !structType->findDecoration<IRExternCppDecoration>()) | ||
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. Instead of skipping here, we should just make sure we never generate the work to emit the struct from the beginning. See
That should result |
||
{ | ||
return; | ||
} | ||
|
||
ensureTypePrelude(structType); | ||
|
||
// If the selected `struct` type is actually an intrinsic | ||
|
@@ -4064,11 +4090,20 @@ void CLikeSourceEmitter::emitGlobalVar(IRGlobalVar* varDecl) | |
|
||
m_writer->emit("\n"); | ||
emitType(varType, initFuncName); | ||
m_writer->emit("()\n{\n"); | ||
m_writer->indent(); | ||
emitFunctionBody(varDecl); | ||
m_writer->dedent(); | ||
m_writer->emit("}\n"); | ||
|
||
// When emiting header, emit only declaration. | ||
if (shouldEmitOnlyHeader()) | ||
{ | ||
m_writer->emit(";\n"); | ||
} | ||
else | ||
{ | ||
m_writer->emit("()\n{\n"); | ||
m_writer->indent(); | ||
emitFunctionBody(varDecl); | ||
m_writer->dedent(); | ||
m_writer->emit("}\n"); | ||
} | ||
} | ||
} | ||
|
||
|
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.
I think this should be "h,hpp,inc".
I have never seen a file with an extension like "h++" or "hxx".
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.
I just don't want to confuse C
.h, .inc
vs C++ headers.I also think that
.c++
and.h++
is really not common, but maybe.cc
and.hh
can be added.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.
I am fine with treating .h as cpp header because we won't be supporting c as target anytime soon. Treating cpp as the default is likely the desired behavior anyways.