On Sun, Mar 25, 2012 at 6:16 PM, Olof Kindgren <[email protected]> wrote:
>
>
> 2012/3/25 R. Diez <[email protected]>
>>
>> Hi Julius:
>>
>> I'm trying to get the OpenRISC core included in ORPSoCV2 to lint without
>> 'important' warnings with Verilator, as I think it is generally worthwhile.
>> The goal is to write a script that lints the Verilog source code daily with
>> a number of tools (Icarus, Verilator, Xilinx?) in order to keep the code
>> tidy in the long run. But before going that far, I'll see if I can fix most
>> of the warnings that exist today.
>>
>> At the moment, I'm only looking for the most 'important' warnings like
>> this:
>>
>>  verilator \
>>    --lint-only -Wall --error-limit 10000 -Wno-UNUSED -Wno-fatal \
>>    -IORPSOCV2/sim/vlt \
>>    -IORPSOCV2/rtl/verilog/include \
>>    -y rtl/verilog/or1200 \
>>    ORPSOCV2/rtl/verilog/or1200/or1200_top.v
>>
>> The next step would be to lint the whole ORPSoCV2, not just the CPU core.
>>
>> By the way, I haven't found a portable way yet in Verilog in order to get
>> rid of the "unused" warnings. There is nothing like the UNUSED and
>> UNUSED_ALWAYS macros in Visual C++. Most of those warnings come from #ifdef
>> FEATURE constructs, which I think should be replaced with Verilog
>> 'parameter' and 'generate' statements, but that would be a separate
>> discussion.
>>
>> If you think getting rid of so many warnings is worth it, we can get
>> started now. For example, I'm getting this warning for the OpenRISC core
>> included in ORPSoC2:
>>
>>  %Warning-DECLFILENAME: rtl/verilog/or1200/or1200_fpu_intfloat_conv.v:323:
>> Filename 'or1200_fpu_intfloat_conv' does not match MODULE name:
>> or1200_fpu_intfloat_conv_except
>>
>> Could you move module 'or1200_fpu_intfloat_conv_except' to a new file
>> called 'or1200_fpu_intfloat_conv_except'? I've tried that locally and it
>> works. Alternatively, that module could be defined as a submodule of the
>> main one, but that's only available for SystemVerilog (2005). Do we need to
>> support older Verilog standards?
>>
>> The warning above isn't really an 'important' warning, but you cannot turn
>> just this single one off in Verilator without disabling others in the same
>> group. If we leave such warnings behind, our eyes will get used to ignoring
>> many warnings on every compilation and may oversee more important ones in
>> the future.

Hi Ruben

This is a good exercise, thanks for having a go. I recall I did a bit
of linting a while back and got through the CPU and Mohor debug IF.
Note that I believe there's still a couple of Verilator lint-off
directive comments in the code here and there.

>>
>> Best regards,
>>  Ruben
>> _______________________________________________
>> OpenRISC mailing list
>> [email protected]
>> http://lists.openrisc.net/listinfo/openrisc
>
>
> I like the efforts put into reducing the number of warnings. It makes it
> much easier to find the real issues, and I have sent a few patches that
> takes care of certain warnings too. I think however that you need to add a
> way to waive some of the warnings (just filter through grep or something),
> as it will be impossible to get rid of all warnings for certain
> combinations, or would require rewriting the code in a way that actually
> makes it less readable. Also, I have seen many times that when you get rid
> of a warning for one tool, another might issue a new warning instead which
> makes it a bit of whack-a-mole.

One I've found annoying width mismatch warnings with string
comparisons in the Verilog (I'll have a parameter MY_OPTION and be
going something like if MY_OPTION == "NONE".) This is because the
lengths are not exactly the same. But the tool usually provides rather
specific warning disable flags such as -Wno-width or something,
although that could be masking other real problems.

Cheers

Julius
_______________________________________________
OpenRISC mailing list
[email protected]
http://lists.openrisc.net/listinfo/openrisc

Reply via email to