On Mon, 16 Oct 2023 10:34:37 GMT, Emanuel Peter <epe...@openjdk.org> wrote:
>> @vnkozlov asked me to guard some debug AD file rules in `#ifdef ASSERT`. >> https://github.com/openjdk/jdk/pull/14785#discussion_r1349391130 >> >> We discovered that the `ASSERT` and `PRODUCT` are not yet passed to ADLC, >> and hence they are always considered `undefined`. Hence, all of these >> `ifdef` blocks would always be ignored. >> >> **Solution** >> I added the flags to `make/hotspot/gensrc/GensrcAdlc.gmk`, just like in >> `make/hotspot/lib/JvmFlags.gmk`. >> >> As @erikj79 commented: we should probably unify this. But I leave that to >> the build team. >> >> **Testing** >> With this code you can see what flags are passed to ADLC: >> >> --- a/src/hotspot/share/adlc/main.cpp >> +++ b/src/hotspot/share/adlc/main.cpp >> @@ -56,6 +56,11 @@ int main(int argc, char *argv[]) >> // Check for proper arguments >> if( argc == 1 ) usage(AD); // No arguments? Then print usage >> >> + for( int i = 1; i < argc; i++ ) { // For all arguments >> + char *s = argv[i]; // Get option/filename >> + fprintf(stderr, "ARGV[%d] %s\n", i, s); >> + } >> + >> // Read command line arguments and file names >> for( int i = 1; i < argc; i++ ) { // For all arguments >> char *s = argv[i]; // Get option/filename >> >> >> On `linux-x64` I get: >> >> ARGV[1] -q >> ARGV[2] -T >> ARGV[3] -DLINUX=1 >> ARGV[4] -D_GNU_SOURCE=1 >> ARGV[5] -g >> ARGV[6] -DAMD64=1 >> ARGV[7] -D_LP64=1 >> ARGV[8] -DNDEBUG >> ARGV[9] -DPRODUCT >> >> >> And on `linux-x64-debug` I get: >> >> ARGV[1] -q >> ARGV[2] -T >> ARGV[3] -DLINUX=1 >> ARGV[4] -D_GNU_SOURCE=1 >> ARGV[5] -g >> ARGV[6] -DAMD64=1 >> ARGV[7] -D_LP64=1 >> ARGV[8] -DASSERT >> >> >> I verified that the `#ifdef` work as expected, by adding this code to >> `src/hotspot/cpu/x86/x86.ad`: >> >> #ifdef ASSERT >> #ifdef PRODUCT >> control >> #endif >> #endif >> >> #ifdef ASSERT >> xxx >> #endif >> >> #ifdef PRODUCT >> yyy >> #endif >> >> When compiling, I get complaints for `yyy` on `linux-x64` and for `xxx` on >> `linux-x64-debug`. But since `ASSERT` and `PRODUCT` never occur together, we >> never get complaints about `control`. >> >> **Running tier1-3 and stress testing ...** > > Emanuel Peter has updated the pull request incrementally with one additional > commit since the last revision: > > add comments like Vladimir requested Marked as reviewed by erikj (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/16178#pullrequestreview-1680021787
