-
Notifications
You must be signed in to change notification settings - Fork 82
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
refactor chess interface #1775
refactor chess interface #1775
Conversation
To properly support other architectures, we need to call xchesscc with --aiearch as a parameter. This makes it more awkward to used the chess-clang hack. It was always a hack anyway, so remove it and instead require users to pass "+Wclang,xir" when feeding LLVMIR to chess. Lastly, clean up a small issue where we really should be using chess-llvm-link to link together downgraded LLVMIR for chess, rather than mixing and matching our LLVM tools with chess tools.
# The path below is cheating a bit since it refers directly to the AIE1 | ||
# version of llvm-link, rather than calling the architecture-specific | ||
# tool version. | ||
opts.aietools_path + "/tps/lnx64/target/bin/LNa64bin/chess-llvm-link", |
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.
So it does not matter which version is used?
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 different synopsys versions are currently pretty close to one another, and 'HEAD' is diverging relatively quickly... This is at some level kicking the can down the road, but it's better than what we have.
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.
Thanks for the generalization.
elif self.opts.link: | ||
await self.do_call(task, ["xchesscc_wrapper", aie_target.lower(), "+w", self.prepend_tmp("work"), "-c", "-d", "-f", "+P", "4", file_core_llvmir_chesslinked, "-o", file_core_obj]) | ||
await self.do_call(task, ["xchesscc_wrapper", aie_target.lower(), "+w", self.prepend_tmp("work"), "-c", "-d", "+Wclang,-xir", "-f", file_core_llvmir_chesslinked, "-o", file_core_obj]) |
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.
While you are here you could add a comment about what these options do without having to dive into Synopsys documentation since it is pretty ad-hoc.
Actually this is repeated 3 times, so perhaps a function?
# Carefully crafted path so that we can inject other scripts into the chess path, namely chess-clang | ||
export PATH=$DIR:$AIETOOLS/bin/unwrapped/lnx64.o:$AIETOOLS/tps/lnx64/target/bin/LNa64bin | ||
$UNWRAPPED_XCHESSCC -p me -C Release_LLVM -D__AIENGINE__ $EXTRA_DEFS -Y clang=$DIR/chess-clang -P $LIBDIR -d -f $@ | ||
xchesscc --aiearch ${AIEARCH} -p me -C Release_LLVM -D__AIENGINE__ $EXTRA_DEFS -P $LIBDIR -d -f $@ |
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.
Adding some comments for simple mortals wanting to understand?
# install too! | ||
foreach(file ${FILES}) | ||
install(PROGRAMS ${file} DESTINATION ${SCRIPT_INSTALL_PATH}) | ||
endforeach() | ||
endif() |
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.
Missing EOL.
Curious, this project has an .editorconfig
stating
insert_final_newline = true
|
||
TARGETVAR=TARGET_${TARGET}_LIBDIR | ||
LIBDIR=${!TARGETVAR} | ||
LIBDIR=${AIETOOLS}/data/${AIEARCH}/lib | ||
|
||
DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" |
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.
Useless line now?
To properly support other architectures, we need to call xchesscc with --aiearch as a parameter. This makes it more awkward to used the chess-clang hack. It was always a hack anyway, so remove it and instead require users to pass "+Wclang,xir" when feeding LLVMIR to chess.
Lastly, clean up a small issue where we really should be using chess-llvm-link to link together downgraded LLVMIR for chess, rather than mixing and matching our LLVM tools with chess tools.