[SYCL][FPGA] Add support for loop count attribute. #3438

zahiraam · 2021-03-29T12:06:00Z

This patch adds support of loop_min, loop_max and loop_avg attributes. They specify the minimum, maximum, or average number of iterations of a for loop.

Signed-off-by: Zahira Ammarguellat <[email protected]>

clang/include/clang/Basic/Attr.td

clang/test/SemaSYCL/intel-fpga-loops.cpp

Signed-off-by: Zahira Ammarguellat <[email protected]>

clang/include/clang/Basic/Attr.td

clang/include/clang/Basic/AttrDocs.td

clang/include/clang/Sema/Sema.h

clang/lib/Sema/SemaStmtAttr.cpp

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam · 2021-04-14T11:54:24Z

I need to write the loop_count_min and loop_count_max too. I am trying to find out if there is a way to represent this attribute as a single one, loop_count with option min, max, avg instead of having an entry for each one.

smanna12 · 2021-04-14T12:10:32Z

I need to write the loop_count_min and loop_count_max too. I am trying to find out if there is a way to represent this attribute as a single one, loop_count with option min, max, avg instead of having an entry for each one.

I think you can use Accessors here like
let Accessors =
[Accessor<"isMin", [CXX11<"intel", "loop_count_min">]>];

Same for Max or Avg

This was you can represent this attribute as a single one.
}

@AaronBallman, what is your opinion?

zahiraam · 2021-04-14T12:18:56Z

I need to write the loop_count_min and loop_count_max too. I am trying to find out if there is a way to represent this attribute as a single one, loop_count with option min, max, avg instead of having an entry for each one.

I think you can use Accessors here like
let Accessors =
[Accessor<"isMin", [CXX11<"intel", "loop_count_min">]>];

Same for Max or Avg

This was you can represent this attribute as a single one.
}

@AaronBallman, what is your opinion?

It looks like it is the way to go.

Signed-off-by: Zahira Ammarguellat <[email protected]>

AaronBallman · 2021-04-14T12:40:55Z

I need to write the loop_count_min and loop_count_max too. I am trying to find out if there is a way to represent this attribute as a single one, loop_count with option min, max, avg instead of having an entry for each one.

I think you can use Accessors here like
let Accessors =
[Accessor<"isMin", [CXX11<"intel", "loop_count_min">]>];
Same for Max or Avg
This was you can represent this attribute as a single one.
}
@AaronBallman, what is your opinion?

It looks like it is the way to go.

Yeah, I think that would be the right approach.

Signed-off-by: Zahira Ammarguellat <[email protected]>

smanna12 · 2021-04-26T19:15:10Z

@zahiraam, could you please update title and add brief summary about the support. Thanks.

zahiraam · 2021-04-27T11:28:09Z

Review please?

smanna12

LGTM. @AaronBallman, could you please take a look at the support? Thanks.

AaronBallman

Can you update the review title and description (this adds multiple attributes, none of which are named loop_count)?

clang/include/clang/Basic/Attr.td

AaronBallman · 2021-04-27T11:45:09Z

clang/include/clang/Basic/AttrDocs.td

+  let Category = DocCatVariable;
+  let Heading = "intel::loop_count_min, intel::loop_count_max, intel::loop_count_avg";
+  let Content = [{
+The "loop_count" attribute specifies the minimum, maximum, or average number of


Suggested change

The "loop_count" attribute specifies the minimum, maximum, or average number of

The loop count attributes specify the minimum, maximum, or average number of

clang/include/clang/Basic/AttrDocs.td

clang/lib/CodeGen/CGLoopInfo.h

clang/lib/Sema/SemaStmtAttr.cpp

Signed-off-by: Zahira Ammarguellat <[email protected]>

clang/include/clang/Basic/AttrDocs.td

clang/lib/CodeGen/CGLoopInfo.cpp

AaronBallman · 2021-04-27T18:49:56Z

clang/lib/CodeGen/CGLoopInfo.h

+  void setSYCLIntelFPGAVariantCount(const char *var, unsigned int C) {
+    StagedAttrs.SYCLIntelFPGAVariantCount.push_back({var, C});


Suggested change

void setSYCLIntelFPGAVariantCount(const char *var, unsigned int C) {

StagedAttrs.SYCLIntelFPGAVariantCount.push_back({var, C});

void setSYCLIntelFPGAVariantCount(const char *Var, unsigned int Count) {

StagedAttrs.SYCLIntelFPGAVariantCount.push_back({Var, Count});

clang/lib/CodeGen/CGLoopInfo.cpp

clang/lib/CodeGen/CGLoopInfo.h

Signed-off-by: Zahira Ammarguellat <[email protected]>

AaronBallman

LGTM aside from a super small grammar nit with the documentation.

clang/include/clang/Basic/AttrDocs.td

Signed-off-by: Zahira Ammarguellat <[email protected]>

AaronBallman

LGTM!

Signed-off-by: Zahira Ammarguellat <[email protected]>

premanandrao

LGTM

The original implementation of this attribute in intel#3438, did not support the [[intel::loop_count()]] attribute spelling. This patch adds support for that spelling. Signed-off-by: Soumi Manna <[email protected]>

The original implementation of this attribute in #3438, did not support the [[intel::loop_count()]] attribute spelling. This patch adds support for that spelling. Signed-off-by: Soumi Manna <[email protected]>

zahiraam added 12 commits March 29, 2021 08:02

[SYCL] Implementation of loop attribute control_avg.

488dfb9

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

b0f255e

Fixing Attr name

0324f5f

Signed-off-by: Zahira Ammarguellat <[email protected]>

Complete implementation

a3a8613

Signed-off-by: Zahira Ammarguellat <[email protected]>

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

5abbbe6

Adding test cases

48d6df0

Signed-off-by: Zahira Ammarguellat <[email protected]>

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

61fba57

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

99cb85f

Indent

44b8681

Signed-off-by: Zahira Ammarguellat <[email protected]>

Indent

7622568

Signed-off-by: Zahira Ammarguellat <[email protected]>

Indent

c3d8c59

Signed-off-by: Zahira Ammarguellat <[email protected]>

CodeGen impl

4dba2ea

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam requested a review from smanna12 April 12, 2021 16:24

smanna12 reviewed Apr 12, 2021

View reviewed changes

clang/include/clang/Basic/Attr.td Outdated Show resolved Hide resolved

smanna12 reviewed Apr 12, 2021

View reviewed changes

clang/test/SemaSYCL/intel-fpga-loops.cpp Outdated Show resolved Hide resolved

smanna12 reviewed Apr 12, 2021

View reviewed changes

clang/test/SemaSYCL/intel-fpga-loops.cpp Outdated Show resolved Hide resolved

zahiraam added 4 commits April 12, 2021 12:55

After review comments

50a06c7

Signed-off-by: Zahira Ammarguellat <[email protected]>

Changing name

1bb84bc

Signed-off-by: Zahira Ammarguellat <[email protected]>

Format

40a3279

Signed-off-by: Zahira Ammarguellat <[email protected]>

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

1f84add

AaronBallman reviewed Apr 13, 2021

View reviewed changes

After review

6ab3b07

Signed-off-by: Zahira Ammarguellat <[email protected]>

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

a5ad650

Formatting

fd34d55

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam added 2 commits April 14, 2021 09:38

Formatting

d812fa4

Signed-off-by: Zahira Ammarguellat <[email protected]>

Looks like the HasCustomTypeTransform is needed

fd74842

Signed-off-by: Zahira Ammarguellat <[email protected]>

Indentation

9d46fc0

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam marked this pull request as ready for review April 25, 2021 01:16

zahiraam requested review from elizabethandrews and premanandrao as code owners April 25, 2021 01:16

zahiraam changed the title ~~Control avg attr~~ Adding loop control count attribute. Apr 26, 2021

zahiraam changed the title ~~Adding loop control count attribute.~~ Adding loop_count attribute. Apr 26, 2021

smanna12 previously approved these changes Apr 27, 2021

View reviewed changes

AaronBallman reviewed Apr 27, 2021

View reviewed changes

Merge remote-tracking branch 'remote/sycl' into control-avg-attr

a44e1ce

zahiraam changed the title ~~Adding loop_count attribute.~~ Adding support for loop count attribute. Apr 27, 2021

Fixes after review

1a20635

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam dismissed smanna12’s stale review via 1a20635 April 27, 2021 16:26

Identation

c10bfaf

Signed-off-by: Zahira Ammarguellat <[email protected]>

AaronBallman reviewed Apr 27, 2021

View reviewed changes

Fixes after review

cefac8a

Signed-off-by: Zahira Ammarguellat <[email protected]>

AaronBallman previously approved these changes Apr 27, 2021

View reviewed changes

clang/include/clang/Basic/AttrDocs.td Outdated Show resolved Hide resolved

Fixes after review

6ddf9ce

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam dismissed AaronBallman’s stale review via 6ddf9ce April 27, 2021 19:31

AaronBallman previously approved these changes Apr 27, 2021

View reviewed changes

Fixes after review

d0ddcdd

Signed-off-by: Zahira Ammarguellat <[email protected]>

zahiraam dismissed AaronBallman’s stale review via d0ddcdd April 27, 2021 19:46

smanna12 approved these changes Apr 27, 2021

View reviewed changes

premanandrao approved these changes Apr 28, 2021

View reviewed changes

bader changed the title ~~Adding support for loop count attribute.~~ [SYCL][FPGA] Add support for loop count attribute. Apr 29, 2021

bader merged commit f74b4ef into intel:sycl Apr 29, 2021

smanna12 mentioned this pull request Feb 9, 2022

[SYCL][FPGA] Add support for [[intel::loop_count()]] attribute #5520

Merged

	The "loop_count" attribute specifies the minimum, maximum, or average number of
	The loop count attributes specify the minimum, maximum, or average number of

		void setSYCLIntelFPGAVariantCount(const char *var, unsigned int C) {
		StagedAttrs.SYCLIntelFPGAVariantCount.push_back({var, C});

[SYCL][FPGA] Add support for loop count attribute. #3438

[SYCL][FPGA] Add support for loop count attribute. #3438

Uh oh!

Conversation

zahiraam commented Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zahiraam commented Apr 14, 2021

Uh oh!

smanna12 commented Apr 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zahiraam commented Apr 14, 2021

Uh oh!

AaronBallman commented Apr 14, 2021

Uh oh!

smanna12 commented Apr 26, 2021

Uh oh!

zahiraam commented Apr 27, 2021

Uh oh!

smanna12 left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronBallman Apr 27, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

premanandrao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zahiraam commented Mar 29, 2021 •

edited

Loading

smanna12 commented Apr 14, 2021 •

edited

Loading