STM32G4 CORDIC HAL Library HAL_CORDIC_Calculate function

[Garnett.Robert]

Hi,

There would appear to be an error in the HAL_CORDIC_Calculate that results in errors when gcc optimsation O3 is turned on.

I had the following code:

void CORDIC_SineCalcSingleValue(float radiansNorm, float sin)
{
	float angle_q31f;
	int32_t angle_q31[1];

	/* Convert radians normalised over pi to Q31 format */
	angle_q31f = radiansNorm * Q_31;
	angle_q31[0] = (int32_t)angle_q31f; // Convert to Q31 integer format
	int32_t result_q31[1];							// Output array Cos then Sin

	/* Calc sine and cos with CORDIC */
	if(HAL_CORDIC_Calculate(&hcordic, angle_q31, result_q31, 1, 500) != HAL_OK)  /* HAL_MAX_DELAY */
	{
		// Handle error
		return;
	}

//	/* Compiler barrier to prevent reordering */
//	__asm volatile ("" ::: "memory");
	sin = (float)result_q31[0] / Q_31; // Convert back to float
}

My theory is the following although nit is probably wrong.

This code fails because the HAL_CORDIC_Calculate  argument result_q31 is not specified as volatile by the HAL function so the function is "skipped" and it does not return so the if function always executes the

// Handle Error

return;

when the O3 optimisation is on even though.the function returns HAL_OK  By putting a memory barrier in before the sine = ...  the HAL_CORDIC_Calculate is forced to fully evaluate in the order written so that is a fix.  But the real issue is the HAL_CORDIC_Calculate should use a volatile for the result arg as the compiler seems to miss the fact the variable is modified by code when it reads the output register: It seems to think it is changed by the peripheral directly.

HAL_StatusTypeDef HAL_CORDIC_Calculate(
CORDIC_HandleTypeDef *hcordic,
int32_t *pInBuff, 
volatile int32_t *pOutBuff,
uint32_t NbCalc, uint32_t Timeout)

/**
  * @brief  Read output data of CORDIC processing, and increment output buffer pointer.
  * @PAram  hcordic pointer to a CORDIC_HandleTypeDef structure that contains
  *         the configuration information for CORDIC module.
  * @PAram  ppOutBuff Pointer to pointer to output buffer.
  * @retval none
  */
static void CORDIC_ReadOutDataIncrementPtr(const CORDIC_HandleTypeDef *hcordic, int32_t **ppOutBuff)
{
  /* First read of output data from the Read Data register */
  **ppOutBuff = (int32_t)READ_REG(hcordic->Instance->RDATA);

  /* Increment output data pointer */
  (*ppOutBuff)++;

  /* Check if second read of output data is expected */
  if (HAL_IS_BIT_SET(hcordic->Instance->CSR, CORDIC_CSR_NRES))
  {
    /* Second read of output data from the Read Data register */
    **ppOutBuff = (int32_t)READ_REG(hcordic->Instance->RDATA);

    /* Increment output data pointer */
    (*ppOutBuff)++;
  }
}

When the function is called the result variable supplied must of course be volatile.

oid CORDIC_SineCalcSingleValue(float radiansNorm, float sin)
{
	float angle_q31f;
	int32_t angle_q31[1];

	/* Convert radians normalised over pi to Q31 format */
	angle_q31f = radiansNorm * Q_31;
	angle_q31[0] = (int32_t)angle_q31f; // Convert to Q31 integer format
	volatile int32_t result_q31[1];							// Output array Cos then Sin

	/* Calc sine and cos with CORDIC */
	if(HAL_CORDIC_Calculate(&hcordic, angle_q31, result_q31, 1, 500) != HAL_OK)  /* HAL_MAX_DELAY */
	{
		// Handle error
		return;
	}

	sin = (float)result_q31[0] / Q_31; // Convert back to float
}

When the volatile qualifier is used the function works correctly with gcc O3 optimisation.

The entire file for this working code is attached below.

Nay thoughts would be appreciated.

[Saket_Om]

Hello @Garnett.Robert 

The output parameters in your functions should be a pointer:

void CORDIC_CosineCalcSingleValue(float radiansNorm, float cos, float sin )
{
	float angle_q31f;
	int32_t angle_q31[1];

	/* Convert radians normalised over pi to Q31 format */
	angle_q31f = radiansNorm * Q_31;
	angle_q31[0] = (int32_t)angle_q31f; // Convert to Q31 integer format
	int32_t result_q31[2];							// Output array Cos then Sin

	/* Calc sine and cos witt CORDIC */
	if(HAL_CORDIC_Calculate(&hcordic, angle_q31, result_q31, 1, 500) != HAL_OK)  /* HAL_MAX_DELAY */
	{
		// Handle error
		return;
	}

	/* Compiler barrier to prevent reordering */
	__asm volatile ("" ::: "memory");
	cos = (float)result_q31[0] / Q_31; // Convert back to float
	sin = (float)result_q31[1] / Q_31; // Convert back to float
}