Closed seldridge closed 6 years ago
colinschmidt
commented
6 years ago Yeah I agree with non-permissive being the default.
I'm fine using this instead of my PR, as that was thrown together rather quickly to get stuff working.
I think it would be good to work through the implications of having to add the -- into rocket-chip and other build scripts. I don't see it as being a problem but just want to double-check.
seldridge
commented
6 years ago @ccelio: Possibly good to get your feedback here, too.
ccelio
commented
6 years ago This looks like a good option that makes the intent clear as to who gets what arguments.
I think I prefer a permissive-off solution that doesn't leave error messages lying around. Other users will not have read this thread and will be confused by scary messages. Also, "permissive-off" makes it clearer that the off part is the same handling routine as the rest of the permissive arguments, whereas "--" doesn't tell me who's responsibility that is.
seldridge
commented
6 years ago Thanks, @ccelio. I think that there's two competing motivations here. -- is POSIX compliant and does what we want. However, -- isn't something that one tends to come across that often even when using Unix-like systems for some time. One of the main aims of this whole argument refactor was originally to make it clear to inexperienced users what was going on with the emulator. Putting a barrier of knowledge of POSIX in there would likely be counterproductive. +permissive-off or some variant there-of isn't POSIX compliant (but none of the + arguments are anyway). However, that technically gives more freedom in that it can be used to bound VCS arguments and resume non-permissive parsing. Additionally, as @colinschmidt hints at, there may be implications in using -- for other files/libraries that are parsing, e.g., emulator.cc (though there's zero need to use permissive parsing with the emulator).
@ccelio -- there's similarly some motivation to remove the POSIX/GNU -/-- arguments as VCS will provide errors for any of those. That bothers me as it's a lot safer to rely on getopt for doing the parsing as opposed to doing the bespoke stuff for the + args, but oh well.
I would highly, highly prefer if this whole thing was POSIX compliant with GNU extensions, but SystemVerilog isn't with it's standardized plusargs.
@colinschmidt -- do you share @ccelio's preference here?
Any other comments or am I bikeshedding?
seldridge
commented
6 years ago Okay, I've updated this to user +permissive/+permissive-off. This has the added benefit of finer control over what you're doing than --. If the user wants, they can technically treat these as un-nestable brackets for enclosing VCS arguments. This has the added benefit of this approach is that -- still works if a user wants to use it. However, I'm not documenting that in the help text as this is something that a user should know about POSIX and not something we need to explicitly state (it also avoids confusion and pushes people towards the more understandable +permissive-off).
I went ahead and added visible usage/help text for HTIF specifically. If you're using the emulator it will scoop up -h and --help before it gets to HTIF making HTIF help text technically unreachable from emulator.cc. This is intentional as HTIF exposes back to emulator.cc what it supports through its header. VCS will whine about -h or --help with a warning, but you still get HTIF explaining what it supports and providing pointers for what it cannot know about (emulator/VCS options, Rocket PlusArg options, etc.).
There are examples of the failure modes that I tested below.
> ./simv-freechips.rocketchip.system-DefaultConfig +verbose $RISCV/riscv64-unknown-elf/share/riscv-tests/isa/rv64ui-p-add
terminate called after throwing an instance of 'std::runtime_error'
what(): could not open +verbose (did you misspell it? If VCS, did you forget +permissive/+permissive-off?)> ./simv-freechips.rocketchip.system-DefaultConfig +permissive +verbose $RISCV/riscv64-unknown-elf/share/riscv-tests/isa/rv64ui-p-add
terminate called after throwing an instance of 'std::invalid_argument'
what(): No binary specified (Did you forget it? Did you forget '+permissive-off' if running with +permissive?)+permissive multiple times
./simv-freechips.rocketchip.system-DefaultConfig +permissive +permissive +verbse $RISCV/riscv64-unknown-elf/share/riscv-tests/isa/rv64ui-p-add
terminate called after throwing an instance of 'std::invalid_argument'
what(): Found +permissive when already parsing permissively+permissive-off when not parsing permissively
> ./simv-freechips.rocketchip.system-DefaultConfig +permissive-off +verbose $RISCV/riscv64-unknown-elf/share/riscv-tests/isa/rv64ui-p-add
terminate called after throwing an instance of 'std::invalid_argument'
what(): Found +permissive-off when not parsing permissively
> ./simv-freechips.rocketchip.system-DefaultConfig -hWarning-[RT_UO] Unsupported option Unsupported option '-h' is ignored
Chronologic VCS simulator copyright 1991-2013 Contains Synopsys proprietary information. Compiler version H-2013.06-SP1_Full64; Runtime version H-2013.06-SP1_Full64; Mar 5 13:54 2018 Usage: ./simv-freechips.rocketchip.system-DefaultConfig [EMULATOR OPTION]... [VERILOG PLUSARG]... [HOST OPTION]... BINARY [TARGET OPTION]... Run a BINARY on the Rocket Chip emulator.
Mandatory arguments to long optiosn are mandatory for short options too.
EMULATOR OPTIONS Consult emulator.cc if using Verilator or VCS documentation if using VCS for available options. EMUALTOR VERILOG PLUSARGS Consult generated-src/.plusArgs for available options
HOST OPTIONS -h, --help Display this help and exit +permissive The host will ignore any unparsed options up until +permissive-off (Only needed for VCS) +permissive-off Stop ignoring options. This is mandatory if using +permissive (Only needed for VCS) --rfb=DISPLAY Add new remote frame buffer on display DISPLAY +rfb=DISPLAY to be accessible on 5900 + DISPLAY (default = 0) --signature=FILE Write torture test signature to FILE +signature=FILE --chroot=PATH Use PATH as location of syscall-servicing binaries +chroot=PATH
HOST OPTIONS (currently unsupported) --disk=DISK Add DISK device. Use a ramdisk since this isn't +disk=DISK supported
TARGET (RISC-V BINARY) OPTIONS These are the options passed to the program executing on the emulated RISC-V microprocessor. terminate called after throwing an instance of 'std::invalid_argument' what(): User quered htif_t help text
Thanks guys. I'm going to merge this. Anybody else who wants to speak up has two more chances (riscv-tools and rocket-chip PRs).
Alternative to #41.
This adds one additional option for HTIF:
+permissive.Normally, HTIF will treat the first option that it doesn't understand as the target binary. This works for
emulator.ccwhere we control all option parsing, but not for VCS which has a slew of options that are entirely out of our control. E.g., the VCS argument+verbosewill get treated as the binary.When
+permissiveis turned on, any subsequent options will be parsed "permissively." This means that anything that HTIF doesn't understand it ignores. This should cover the entirety of all possible VCS options which have any number of odd formats. See Appendix C here: http://www.cerc.utexas.edu/~deronliu/vlsi1/lab3/2014_fall_VLSI_I/LAB3_Website/lab3a/vcs_tutorial.pdf.This introduces a problem in that HTIF needs to then know what the binary is supposed to be. We could introduce another option (
+permissive-off) which would turn off permissive parsing. However, I think the cleanest approach is to use the getopt-understood--for specifying the end of options. VCS throws a warning for this as it doesn't understand it, but that should be okay. Using an explicit plus-arg (the above+permissive-off) would avoid this.This results in a VCS built emulator call of:
@colinschmidt: thoughts?
Personally, I'd prefer to keep the non-permissive parsing as the default as it avoids user errors (my errors), e.g., my misspelled command line arguments don't get swallowed.
Todo:
+permissive-on/+permissive-offvs.+permissive/--