Avoiding unused-but-set warning

Go To Last Post
18 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have a function that initializes the SPI hardware, as part of that initialization I want to read the SPI data register, so as to ensure the SPIF flag is cleared, for example.

void SPI_Init(void)
{
void uint8_t temp;
blah;
blah;
blah;
temp = SPID_DATA;
blah:
}

Is there a trick I can use to avoid getting a "variable unused but set" warning, or is there a smarter way to do this?

P.S. I don't recall seeing this behaviour with AS5, AS5 or AS5.1, so I guess AS6 is more stringent.

John

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well one thing is that "temp" does not actually have to exist here. The line could literally just say:

SPID_DATA;

with no assignment and (assuming SPID_DATA is a volatile source) the compiler will write code to make a read access anyway.

For example:

int main(void) {
	while(1) {
		PIND;
	}		
}

generates:

int main(void) {
	while(1) {
		PIND;
  94:	80 b3       	in	r24, 0x10	; 16
  96:	fe cf       	rjmp	.-4      	; 0x94 

I would of course include a comment to say what the line ios doing such as "// force read of register" as the maintainer may wonder what's going on. Another technique is to use:

	while(1) {
		(void)PIND;
	}		

where the (void) cast hopefully tells the reader "this is being read but discarded".

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

BTW to come back to your original question if you want to keep "temp". You may have noticed that the GCC errors are quite clever these days in that they often tell you why they are there and by implication how to stop them. The warning you have here is:

variable 'temp' set but not used [-Wunused-but-set-variable]

that bit in square brackets is telling you why you got the warning. With most -W switches you can quell them by adding no- to the start. So you could try -Wno-unused-but-set-variable

EDIT: confirmed, that does quell this warning.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks, Cliff.

SPI_DATA is a volatile, as are all SFR's, AFAIK.

Your first method would be preferable, is it portable or a idiom of GCC specifically?

I understand that I could supress the warning, but I would rather not, as I often end up with unused local variables and it's nice to have the GCC find them for me.

John

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Your first method would be preferable, is it portable or a idiom of GCC specifically?

Should be portable. I think any compiler will instigate a read for a volatile symbol.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Should be portable. I think any compiler will instigate a read for a volatile symbol.

I agree.

I was bitten by this a few weeks ago, and vowed never to be caught again.

Not all compilers will generate code for:

     SPDR;      // empty buffer
     SPSR;      // clear flags

This should always be ok:

     dummy = SPDR;      // empty buffer
     dummy = SPSR;      // clear flags

The compiler will probably generate the same code anyway. After all, the AVR can only read an SFR into a machine register. If you know that you are using avr-gcc, you know that either is acceptable.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

The compiler will probably generate the same code anyway

Yes but that brings us back to John's problem. Those lines with the avr-gcc in AS6 will warn that dummy is assigned but not subsequently used. I suppose that making "dummy" volatile would work as the compiler is then compelled to do the read/assignment but it'll generate un-necessary code.

EDIT: nope volatile still tells us it's set/unused in which case I guess the variable needs global scope? This works without GCC warning:

uint8_t temp;

int main(void) {
	while(1) {
		temp = PIND;
	}		
}

but generates:

		temp = PIND;
  94:	80 b3       	in	r24, 0x10	; 16
  96:	80 93 60 00 	sts	0x0060, r24
  9a:	fc cf       	rjmp	.-8      	; 0x94 

doing an unnecessary STS.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmmm....
Maybe I'll stick with the temp or dummy, and ignore the warning.
If I always use dummy, for example, then it'll be easy to spot.

John

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I just tried it with avr-gcc.

  uint8_t dummy;
  dummy = PIND;
  PIND;
  (void)PIND;

All these statements produce exactly the same ASM statements. i.e.

   IN   r24, 0x10

The dummy= version gives the 'variable set but not used' warning.
Since you are using avr-gcc, you can be confident that the other two statements produce the same code.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
Since you are using avr-gcc, you can be confident that the other two statements produce the same code.
And if you are using a compiler that does not produce the same code, then it is time to look for another compiler.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have found another solution, on which I would appreciate opinions.
I have rewritten the initialization function so that it returns a uint8_t.
I thought the new, improved GCC might moan about the function call discarding the value, but it doesn't, and the variable set but unused warning also disappears. As far as I can tell, there is no extra code produced, as r24 is used, and that's where the return value resides.

John

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ezharkov wrote:
david.prentice wrote:
Since you are using avr-gcc, you can be confident that the other two statements produce the same code.
And if you are using a compiler that does not produce the same code, then it is time to look for another compiler.

I have already indicated that I'm using GCC, added to which the thread is in the "AVR Studio 6 and Atmel Studio 6" forum, so I'm not entirely sure what point you are making.

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

John_A_Brown wrote:
I have already indicated that I'm using GCC, added to which the thread is in the "AVR Studio 6 and Atmel Studio 6" forum, so I'm not entirely sure what point you are making.
Rereading my post, yes, I agree, it is hard to tell what point I was making. Sorry about that.

What I had in mind when I made the post was:
- the very first thing that Cliff suggested was a portable solution, not something specific to GCC;
- if some other compiler does not support that, then it is probably not a very good compiler.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ezharkov wrote:
John_A_Brown wrote:
I have already indicated that I'm using GCC, added to which the thread is in the "AVR Studio 6 and Atmel Studio 6" forum, so I'm not entirely sure what point you are making.
Rereading my post, yes, I agree, it is hard to tell what point I was making. Sorry about that.

What I had in mind when I made the post was:
- the very first thing that Cliff suggested was a portable solution, not something specific to GCC;
- if some other compiler does not support that, then it is probably not a very good compiler.


OK, I understand. It seems that David does not agree, as he says that not all compilers would generate code for:
SPID_DATA:

For the time being I will stick with my "bodge" of specifying a return value.

Four legs good, two legs bad, three legs stable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't think it's a bodge for GCC at least because, as you say it would be using R24 anyway but if this is for a portable solution you'd have to look at what other compilers did with that as you might be back to redundant code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I agree that a C compiler should not optimise a read of a volatile location. e.g. SPDR;

avr-gcc, CodeVision, are all ok. I would guess IAR is too.

Rowley, ImageCraft.v7 are not ok.

Personally, I believe in using straightforward source code. I am sure that you can trip up many Compilers with some obscure pieces of legal C.

If there was some dramatic efficiency gained by the 'SPDR;' construct, I would be annoyed by the non-compliance. (I am fairly certain that SPDR; is legal C)

I don't normally attempt to use SPDR; style. However I had used in a program a few weeks ago. I must have been too idle to type dummy =

I could not understand why the program worked with CV and avr-gcc but failed with Rowley. I wasted several hours of grief. I do not intend to repeat the experience.

Ok. You may always use one brand of Compiler today. But no-one can be sure of the future.

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
I could not understand why the program worked with CV and avr-gcc but failed with Rowley.
Not that I have any plan to switch to Rowley, just curious - what exactly did it do? Just ignored the whole statement, without any warnings?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes. No ASM was generated by both Rowley and ImageCraft.

Rowley:

Quote:

c:/src/gnuavr/c_code.c(9): warning W2031: reference to 'volatile unsigned char' removed

ImageCraft.v7

Quote:

!W C:\src\gnuavr\c_code.c(9):[warning] reference to `volatile unsigned char' elided
!W C:\src\gnuavr\c_code.c(9):[warning] expression with no effect elided

As I said. I am sure that this is non-compliance. However I am no C expert. Nor do I intend to worry about it.

David.