> On 2011-09-21 07:50:10, Steve Reinhardt wrote: > > Were there places where the ambiguity between a structure field access and > > the type notation were giving you problems? Would they have been fixed > > just by making the one change here where the regex matches only valid type > > extensions and not any sequence of characters after a dot? > > > > My reluctance here is that you're substituting one ambiguous overloaded > > syntax for another... it's unlikely that I have an identifier that by > > itself ends in "_uq" but it seems equally unlikely that I've got a > > structure field named "uq". Between the two, IMO, the dot syntax is > > slightly more meaningful since the type qualifiers are a little bit like > > union fields. > > > > It's fine to me to change it, but it seems like we should change it to > > something that's clearly less ambiguous; how about colon? There's some > > potential ambiguity with ?:, but that's not a very common operator, and > > even when there's potential ambiguity it would be easy to fix by requiring > > spaces around the : when you don't want it to be treated as a type > > qualifier. > > > > Some other possibilities are #, `, \, ~, and !. I think only backtick is > > completely unused, but the others should be pretty unambiguous when > > embedded in an identifier.
There was an instance of a sw field and an sw type which showed up even now that accessing members with . is so cumbersome. Really, though, I think using . for both is not only potentially ambiguous, it's also going to be really confusing. People won't be able to look at a bit of code, say Control.udw, and know for sure without looking something up if udw is a type or a member. Also the regular expressions in, say, the reg_or_imm sort of multiplexing instruction formats don't know what the type suffixes are. They just accept anything like they did before, but it happens to work out. Even though _ may at first seem like it would blend in too much, and/or be ambiguous, I think after looking at some examples it isn't really. Historically operand names have been capitalized, so putting a _ is a clear separator. It's also subtle enough to not obscure the name itself. I actually had a few ISAs converted over to using @ before, and it was just so ugly I gave up on it. You basically end up with this big wart in the middle of everything where . or _ stay out of the way. Lets say you have a register XMM in x86 which is a, say, 64 bit floating point register. Normally those are 128 bits, but I split them up into 64 bits. But in any case, lets say the default type is a double, but you want it to be a structure/union which is broken into lanes of various sizes. The type suffix for that could be lns, and the 2 byte lanes could be, for instance, wa, wb, wc, and wd. To store 0xaa55 into the third 2 byte word of the XMM register, it would look like this. XMM_lns.wc = 0xaa55; I think that's clear for a couple reasons. First, it's not uncommon to use a suffix to distinguish between different versions of the same thing with the same name, and that's essentially what _lns is doing. Second, "." means membership in C and C++ and python, etc., and here it does too. You're accessing the wc member, not essentially doing a cast (or really selecting a variant). If you had a control register and you saw. Control.udw = 1 And you weren't already programmed to assume that was a type override, your initial assumption would likely be that you were assigning to the udw field of the control register. In some cases, that may be exactly what you want to do. If you have a (recently demoted) Twin64_t operand like, say, Mem after a load, you could get at its a and b members like this. Ra = Mem_tud.a; Rb = Mem_tud.b; which is also pretty clear. Another nice property of "_" compared to other punctuation, in addition to it being aesthetically nice, is that it can't be confused for an operator or break up a variable name. You could, theoretically, leave the code untouched and it would still be valid code. You can't for logistical reasons I mention in my commit message, but, otherwise it would work just fine. You could copy and paste the code into your own file, and it would just work. This also means the operand detector can be a little dumber because it doesn't have to worry about accidentally thinking Ra/Rb means Ra but instantiated as an Rb type operand. I thought about this quite a bit and it wasn't obvious that this would work out very well, but after some thought and experimentation I think it does. - Gabe ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.m5sim.org/r/873/#review1547 ----------------------------------------------------------- On 2011-09-20 22:59:24, Gabe Black wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.m5sim.org/r/873/ > ----------------------------------------------------------- > > (Updated 2011-09-20 22:59:24) > > > Review request for Default, Ali Saidi, Gabe Black, Steve Reinhardt, and > Nathan Binkert. > > > Summary > ------- > > ISA parser: Use '_' instead of '.' to delimit type modifiers on operands. > > By using an underscore, the "." is still available and can unambiguously be > used to refer to members of a structure if an operand is a structure, class, > etc. This change mostly just replaces the appropriate "."s with "_"s, but > there were also a few places where the ISA descriptions where handling the > extensions themselves and had their own regular expressions to update. The > regular expressions in the isa parser were updated as well. It also now > looks for one of the defined type extensions specifically after connecting "_" > where before it would look for any sequence of characters after a "." > following an operand name and try to use it as the extension. This helps to > disambiguate cases where a "_" may legitimately be part of an operand name but > not separate the name from the type suffix. > > Because leaving the "_" and suffix on the variable name still leaves a valid > C++ identifier and all extensions need to be consistent in a given context, I > considered leaving them on as a breadcrumb that would show what the intended > type was for that operand. Unfortunately the operands can be referred to in > code templates, the Mem operand in particular, and since the exact type of Mem > can be different for different uses of the same template, that broke things. > > > Diffs > ----- > > src/arch/alpha/isa/decoder.isa dee1f3ab92e4 > src/arch/alpha/isa/int.isa dee1f3ab92e4 > src/arch/alpha/isa/main.isa dee1f3ab92e4 > src/arch/arm/isa/insts/branch.isa dee1f3ab92e4 > src/arch/arm/isa/insts/data.isa dee1f3ab92e4 > src/arch/arm/isa/insts/div.isa dee1f3ab92e4 > src/arch/arm/isa/insts/fp.isa dee1f3ab92e4 > src/arch/arm/isa/insts/ldr.isa dee1f3ab92e4 > src/arch/arm/isa/insts/macromem.isa dee1f3ab92e4 > src/arch/arm/isa/insts/mem.isa dee1f3ab92e4 > src/arch/arm/isa/insts/misc.isa dee1f3ab92e4 > src/arch/arm/isa/insts/mult.isa dee1f3ab92e4 > src/arch/arm/isa/insts/neon.isa dee1f3ab92e4 > src/arch/arm/isa/insts/str.isa dee1f3ab92e4 > src/arch/arm/isa/insts/swap.isa dee1f3ab92e4 > src/arch/isa_parser.py dee1f3ab92e4 > src/arch/mips/isa/decoder.isa dee1f3ab92e4 > src/arch/mips/isa/formats/fp.isa dee1f3ab92e4 > src/arch/mips/isa/formats/int.isa dee1f3ab92e4 > src/arch/mips/isa/formats/mem.isa dee1f3ab92e4 > src/arch/power/isa/decoder.isa dee1f3ab92e4 > src/arch/power/isa/formats/fp.isa dee1f3ab92e4 > src/arch/sparc/isa/base.isa dee1f3ab92e4 > src/arch/sparc/isa/decoder.isa dee1f3ab92e4 > src/arch/x86/isa/microops/fpop.isa dee1f3ab92e4 > src/arch/x86/isa/microops/ldstop.isa dee1f3ab92e4 > src/arch/x86/isa/microops/limmop.isa dee1f3ab92e4 > src/arch/x86/isa/microops/mediaop.isa dee1f3ab92e4 > src/arch/x86/isa/microops/regop.isa dee1f3ab92e4 > > Diff: http://reviews.m5sim.org/r/873/diff > > > Testing > ------- > > > Thanks, > > Gabe > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
