[SYCL][FPGA] Refactor of statement attributes

[smanna12]

This patch

1. refactors six statement attributes to align with upstream:

   SYCLIntelFPGALoopCountAttr
   SYCLIntelFPGAInitiationIntervalAttr
   SYCLIntelFPGAMaxConcurrencyAttr
   SYCLIntelFPGAMaxInterleavingAttr
   SYCLIntelFPGASpeculatedIterationsAttr
   SYCLIntelFPGALoopCoalesceAttr

2. stores expression as ConstantExpr in Semantic Attributes

3. removes generic function "BuildSYCLIntelFPGALoopAttr"

4. updates codegen codes to use single variable for loop metadata and value.

Signed-off-by: Soumi Manna [email protected]

[AaronBallman]

Thanks for the refactoring, it looks like it's heading in a good direction! Should you also add more template instantiation test coverage for the improvements in SemaStmtAttr.cpp as well?

[smanna12]

Should you also add more template instantiation test coverage for the improvements in SemaStmtAttr.cpp as well?

Thanks @AaronBallman for the reviews. Do you mean to add AST tests? Are test cases below sufficient here?

void x() {
  // Test that checks expression is not a constant expression.
  // expected-note@+1{{declared here}}
  int a[10];
  int foo;
  // expected-error@+2{{expression is not an integral constant expression}}
  // expected-note@+1{{read of non-const variable 'foo' is not allowed in a constant expression}}
  [[intel::max_concurrency(foo + 1)]] for (int i = 0; i != 10; ++i)
       a[i] = 0;

 // Test that checks expression is a constant expression.
  constexpr int bar = 0;
  [[intel::max_concurrency(bar + 2)]] for (int i = 0; i != 10; ++i) // OK
      a[i] = 0;
}

// Test that checks wrong function template instantiation and ensures that the type
// is checked properly when instantiating from the template definition.
struct S {};
template <typename Ty>
void R() {
  int a[10];
  // expected-error@+2 {{integral constant expression must have integral or unscoped enumeration type, not 'S'}}
  // expected-error@+1 {{integral constant expression must have integral or unscoped enumeration type, not 'float'}}
  [[intel::max_concurrency(Ty{})]] for (int i = 0; i != 10; ++i)
      a[i] = 0;
}

int main() {
     R<S>(); //expected-note {{in instantiation of function template specialization 'R<S>' requested here}}
     R<float>(); // expected-note {{in instantiation of function template specialization 'R<float>' requested here}}
}

}

[AaronBallman]

Should you also add more template instantiation test coverage for the improvements in SemaStmtAttr.cpp as well?

Thanks @AaronBallman for the reviews. Do you mean to add AST tests? Are test cases below sufficient here?

Sema tests ("AST tests" mean -ast-dump tests to me). But yes, the test cases you posted are the kinds I'm hoping to add for these attributes since you're making changes to support them for template instantiations now. One other test to add to round things out is:

template <int N>
void R() {
  int a[10];
  [[intel::max_concurrency(N)]] for (int i = 0; i != 10; ++i)
      a[i] = 0;
}

int main() {
  R<1>();
  R<0>();
  R<-1>();
}

[smanna12]

Should you also add more template instantiation test coverage for the improvements in SemaStmtAttr.cpp as well?

Thanks @AaronBallman for the reviews. Do you mean to add AST tests? Are test cases below sufficient here?

Sema tests ("AST tests" mean -ast-dump tests to me). But yes, the test cases you posted are the kinds I'm hoping to add for these attributes since you're making changes to support them for template instantiations now. One other test to add to round things out is:

template <int N>
void R() {
  int a[10];
  [[intel::max_concurrency(N)]] for (int i = 0; i != 10; ++i)
      a[i] = 0;
}

int main() {
  R<1>();
  R<0>();
  R<-1>();
}

Thanks @AaronBallman. I will add and push new patch here.

[AaronBallman]

Just style nits, but mostly LG.

[AaronBallman]

LGTM, thank you for the cleanup!

[smanna12]

Thank you for the reviews.

[smanna12]

Thank you for the reviews.